Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Add client sided endpoints parser #8580

Merged
merged 255 commits into from
Aug 13, 2024

Conversation

Jan-Thurner
Copy link
Contributor

@Jan-Thurner Jan-Thurner commented May 12, 2024

Checklist

General

Pull Requests

Pull Requests related to the requested feature:
#8407
#8455

Context

The Proposed feature extends the GitHub Action originally created in this PR . The original PR focused on parsing information related to changed server endpoints, a crucial step in ensuring the accuracy of server-side interactions within the Artemis codebase. The proposed extension further enhances this functionality by introducing the parsing of client-sided REST calls. Currently, the focus is on extracting essential information such as the HTTP methods and URLs used in these calls. While the current scope is limited to parsing HTTP methods and URLs, this extension lays the groundwork for future enhancements aimed at improving code quality and development efficiency within the Artemis project.

Problem

Errors, like a mismatch in the URLs of REST calls and endpoints, slipping through the code review and testing process could break features and reduce a system's usability. The problem tackled in this issue is that reviewing client-sided REST calls manually is a time-consuming and error-prone task, potentially leading to:

  • Delayed Development: Time-consuming tasks can slow down the development process, potentially delaying the delivery of new features or updates.
  • Reduced Efficiency: When tasks are error-prone, developers may need to spend additional time fixing mistakes, reducing overall efficiency.
  • Increased Costs: Time is money, as they say. Any inefficiencies in the development process can lead to increased costs, whether it's in terms of developer hours or missed deadlines.
  • Quality Issues: Errors that slip through the review process can lead to bugs or issues in the software, potentially affecting its stability, performance, or security.
  • Negative User Experience: Ultimately, if errors make it into the final product, users may experience disruptions or frustration, leading to a poor user experience and potentially impacting user retention or satisfaction.

Motivation

The motivation behind this feature is to simplify reviewing newly created or modified REST calls. The tedious task of manually parsing out relevant Information regarding a system's REST calls should be automated to increase the efficiency in the reviewing process


Requirements Engineering

Proposed System

The proposed solution is a GitHub action analyzing the client-sided REST calls on every commit. It should use the Library TypeScript Compiler API to build an abstract syntax tree (AST) representing the changed typescript files within each PR. The AST will be traversed and analyzed to identify instances of client-sided REST calls. When a REST call is detected, the system will extract relevant information such as the file path containing the REST call, the kind of REST call and the used URL.

Requirements

Functional requirements:

  • FR1: Identification of new or modified REST calls: The GitHub action must Identify files containing newly created or modified REST calls.
  • FR2: Output of REST calls' file path: The GitHub action must output the identified REST call's file path to the console
  • FR3: Output of REST call's line: The GitHub action must output the line number in which the REST call is performed to the console
  • FR4: Output HTTP method of REST calls: The GitHub action must output the HTTP method (e.g., GET, POST, PUT, DELETE) associated with each REST call to the console
  • FR5: Output of REST call's URL: The GitHub action must output the URL of each identified REST call to the console

Non-functional requirements:

  • NFR1: Performance: The Identification of client-sided REST calls must not take longer than 3 Minutes
  • NFR2: Extendability: The GitHub action should be modular, and each functionality must be in its own method

Analysis

Dynamic Behavior

The following model illustrates the currently flawed code review process. Developers develop a feature, open a PR, and get it reviewed by other developers. If a reviewer discovers one or more flaws in the code, they request changes. Those requested changes are then implemented by the developer, and the PR is re-reviewed. Once every reviewer has approved the PR, maintainers review the PR. If they discover flaws, they also request changes. If not, the PR gets merged. If errors remain undiscovered during the review process, they will now get merged into the codebase and reduce the system's usability.
DynamicModelPR3

Steps for Testing:

The check result of an up-to-date test commit can be found here: https://github.com/ls1intum/Artemis/actions/runs/10125390211. On that page, the result can be downloaded in a .json file.

  1. Push a commit containing changes in a client service file (eg. src/main/webapp/app/course/tutorial-groups/services/tutorial-group-free-period.service.ts)
  2. Navigate to Checks
  3. Open analysis-of-endpoint-connections
  4. navigate to Run analysis-of-endpoint-connections-client
  5. Verify that all Endpoints contained in changed files are in the output.

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked






Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new tool for analyzing TypeScript files to extract REST API call details and manage class structures.
    • Enhanced GitHub Actions workflow for better execution and integration of TypeScript analysis.
  • Chores

    • Improved project configuration with an updated package.json for better dependency management.

Jan-Thurner and others added 30 commits April 16, 2024 10:51
…ing-endpoint-connections' into development/github-action-analyzing-endpoint-connections
Also, change a file containing endpoints and one not containing endpoints for testing.
…s' into development/parsing-of-server-sided-endpoints
@Jan-Thurner Jan-Thurner linked an issue Aug 6, 2024 that may be closed by this pull request
az108
az108 previously approved these changes Aug 6, 2024
Copy link
Contributor

@az108 az108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after Feedback changes :)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Copy link
Contributor

@pzdr7 pzdr7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapprove

Copy link
Contributor

@JohannesStoehr JohannesStoehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapprove

Copy link
Contributor

@JohannesWt JohannesWt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Looks good. Thanks for the changes

@Jan-Thurner Jan-Thurner requested a review from az108 August 8, 2024 09:06
Copy link
Contributor

@az108 az108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reapprove

Copy link
Contributor

@MarkusPaulsen MarkusPaulsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@sarpsahinalp sarpsahinalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapprove, changes look good!

@pzdr7 pzdr7 added the maintainer-approved The feature maintainer has approved the PR label Aug 12, 2024
Copy link
Member

@bassner bassner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please add the requested follow-up changes asap

@bassner bassner added this to the 7.5.1 milestone Aug 13, 2024
@bassner bassner merged commit b67c333 into develop Aug 13, 2024
20 of 24 checks passed
@bassner bassner deleted the feature/development/parsing-client-sided-REST-api branch August 13, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) maintainer-approved The feature maintainer has approved the PR ready to merge
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Implement Parsing out REST Calls on the Client