Skip to content

[Feat][Typescript Angular] Implement deepObject query params (OAS3.0) #21108

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

filiptc
Copy link

@filiptc filiptc commented Apr 17, 2025

Generate correct service implementation for typescript-angular for deepObject style query parameters of type object, as per OAS 3.0 Specification.

  • deepObject – simple non-nested objects are serialized as paramName[prop1]=value1&paramName[prop2]=value2&....

This closes #19342 and allows for the correct implementation of JSON:API pagination and filtering.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.
cc Typescript Committee: @TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov @davidgamero @mkusaka @joscha

@filiptc filiptc force-pushed the feature/typescript-angular-deep-object-query-params branch 2 times, most recently from e949803 to c98e870 Compare April 17, 2025 18:46
@filiptc filiptc force-pushed the feature/typescript-angular-deep-object-query-params branch from c98e870 to d99dba4 Compare April 17, 2025 19:46
@filiptc filiptc force-pushed the feature/typescript-angular-deep-object-query-params branch from d99dba4 to 696442c Compare April 17, 2025 19:58
Copy link
Contributor

@joscha joscha left a comment

Choose a reason for hiding this comment

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

  • add a test somewhere that exercises this codepath and generates a query string with the expected format?

@filiptc
Copy link
Author

filiptc commented Apr 19, 2025

  • add a test somewhere that exercises this codepath and generates a query string with the expected format?

Will do, and push together with the rest of suggestions.

@filiptc
Copy link
Author

filiptc commented Apr 19, 2025

Pushed tests (d2a06dd). Kindly double check, as I wasn't previously familiar with the testing patterns. 🙂

nvm install 16
nvm alias default 16
nvm install 18
nvm alias default 18
Copy link
Author

Choose a reason for hiding this comment

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

Increased Node version as tests were failing due to Angular refusing to compile on NodeJS versions <18. Node 18 is required for all currently supported Angular versions (docs).


// THEN
final String fileContents = Files.readString(Paths.get(output + "/api/default.service.ts"));
assertThat(fileContents).containsOnlyOnce("<any>options, 'options', true);");
Copy link
Member

Choose a reason for hiding this comment

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

are these assertions related to the deepObject case?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, the last parameter is isDeep.

@macjohnny
Copy link
Member

thanks for your contribution!

@@ -0,0 +1,9 @@
generatorName: typescript-angular
outputDir: samples/client/petstore/typescript-angular-v16-provided-in-root/builds/deep-object
Copy link
Member

Choose a reason for hiding this comment

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

how about having it in the newest angular version, so it doesnt get lost when deleting older versions?

Copy link
Author

Choose a reason for hiding this comment

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

Happy to. I took your comment here too literally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] [typescript-angular] Incorrect key for query param keys
3 participants