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

feat: Support basePath in options-per-service for openapi generator - Option 2 #5247

Merged
merged 73 commits into from
Dec 19, 2024

Conversation

KavithaSiva
Copy link
Contributor

@KavithaSiva KavithaSiva commented Dec 6, 2024

Closes SAP/cloud-sdk-backlog#1259.

This approach enables customising the basePath both before and after client generation.

It changes the generated client code to also add an additional basePath and pass it on to the OpenApiRequestBuilder.

export const CollectionsApi = {
  _defaultBasePath: '/base/path/to/service',
  
  createCollection: (body: CollectionRequest) =>
    new OpenApiRequestBuilder<any>(
      'post',
      '/collections',
      {
        body
      },
      // Forced to use the fully qualified name, as `this._default_basePath` results in Object is possibly 'undefined'.
      CollectionsApi._defaultBasePath
    )
};

A new setBasePath() method is also introduced in the OpenApiRequestBuilder.
Usage from a user would look like this:

await CollectionsApi.CreateCollection({
  title,
  embeddingConfig: {},
}).setBasePath(`/some/base/path`).execute(destination);
  • I know which base branch I chose for this PR, as the default branch is v3-main now, which is not for v4 development.
  • If my change will be merged into the main branch (for v4), I've updated (V4-Upgrade-Guide.md)[./V4-Upgrade-Guide.md] in case my change has any implications for users updating to SDK v4

KavithaSiva and others added 30 commits December 2, 2024 20:48
…hub.com:SAP/cloud-sdk-js into feat/openapi-support-basePath-optionsPerService
…ud-sdk-js into feat/openapi-support-setBasePath
…ud-sdk-js into feat/openapi-support-setBasePath
@KavithaSiva KavithaSiva dismissed deekshas8’s stale review December 19, 2024 08:30

I have addressed all review comments.

{
method: 'get',
middleware: [],
url: 'base/path/to/service/test',
Copy link
Contributor

Choose a reason for hiding this comment

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

@tomfrenken Fyi since it relates to some of your code changes like two years ago. Here without a leading slash is the same behaviour as in OData, where remove (leading/trailing) slashes functions were used at almost each and every step to ensure no slashes at all. But it feels kind inconsistent to me because without setting the base path, we use and probably also recommend adding a leading slash at the path.

Copy link
Contributor

@ZhongpinWang ZhongpinWang left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Thanks for this nice contribution.

Be sure to add this to v4.

@KavithaSiva KavithaSiva merged commit 2943cd5 into v3-main Dec 19, 2024
17 checks passed
@KavithaSiva KavithaSiva deleted the feat/openapi-support-setBasePath branch December 19, 2024 12:23
KavithaSiva added a commit that referenced this pull request Dec 20, 2024
… - Option 2 (#5247)

* chore: append basePath to path pattern in openapi generator

* chore: switch to using removeSlashes from util

* chore: add additional test

* chore: check-public-api change

* Changes from lint:fix

* chore: update test

* chore: adjust all tests

* chore: fix codeQL warning

* Changes from lint:fix

* chore: add changeset

* chore: cleanup tests

* feat: extend openapi request builder for basePath

* chore: introduce setBasePath

* feat: introduce _defaultBasePath to the generated files

* chore: fix order of parameters with intro of basepath

* chore: fix tests

* chore: add setBasePath

* chore: fix failing e2e test

* Changes from lint:fix

* chore: cleanup path

* Changes from lint:fix

* chore: adjust test

* Changes from lint:fix

* chore: switch to using undefined for default basePath value

* chore: cleanup test

* chore: update changeset

* chore: adjust changeset

* chore: fix unit test

* chore:cleanup

* chore: fix test

* chore: cleanup changesets

* fix: integration test

* chore: fix path

* chore:fix path

* chore: remove dead code

* chore: review comment

* Update .changeset/chilled-flowers-hide.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/openapi/src/openapi-request-builder.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* chore: address review comments

* chore: revert update to removeSlashes utility

* Changes from lint:fix

* chore: address review comments

* chore: tests fix

* chore: adjust tests

* chore: address review comments

* Changes from lint:fix

* fix: begin all paths using '/'

* chore: address review comments

* Update .changeset/chilled-flowers-hide.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update .changeset/chilled-flowers-freeze.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/util/src/remove-slashes.spec.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/util/src/remove-slashes.spec.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* chore: address review comments

* Changes from lint:fix

* chore: add review changes

* chore: address review comments

* chore:address review comments

* chore: address review comments

* chore: release note fix

---------

Co-authored-by: cloud-sdk-js <[email protected]>
Co-authored-by: Junjie Tang <[email protected]>
Co-authored-by: Deeksha Sinha <[email protected]>
KavithaSiva added a commit that referenced this pull request Dec 20, 2024
@KavithaSiva KavithaSiva mentioned this pull request Dec 20, 2024
2 tasks
KavithaSiva added a commit that referenced this pull request Dec 20, 2024
… - Option 2 (#5247)

* chore: append basePath to path pattern in openapi generator

* chore: switch to using removeSlashes from util

* chore: add additional test

* chore: check-public-api change

* Changes from lint:fix

* chore: update test

* chore: adjust all tests

* chore: fix codeQL warning

* Changes from lint:fix

* chore: add changeset

* chore: cleanup tests

* feat: extend openapi request builder for basePath

* chore: introduce setBasePath

* feat: introduce _defaultBasePath to the generated files

* chore: fix order of parameters with intro of basepath

* chore: fix tests

* chore: add setBasePath

* chore: fix failing e2e test

* Changes from lint:fix

* chore: cleanup path

* Changes from lint:fix

* chore: adjust test

* Changes from lint:fix

* chore: switch to using undefined for default basePath value

* chore: cleanup test

* chore: update changeset

* chore: adjust changeset

* chore: fix unit test

* chore:cleanup

* chore: fix test

* chore: cleanup changesets

* fix: integration test

* chore: fix path

* chore:fix path

* chore: remove dead code

* chore: review comment

* Update .changeset/chilled-flowers-hide.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/openapi/src/openapi-request-builder.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* chore: address review comments

* chore: revert update to removeSlashes utility

* Changes from lint:fix

* chore: address review comments

* chore: tests fix

* chore: adjust tests

* chore: address review comments

* Changes from lint:fix

* fix: begin all paths using '/'

* chore: address review comments

* Update .changeset/chilled-flowers-hide.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update .changeset/chilled-flowers-freeze.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/util/src/remove-slashes.spec.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/util/src/remove-slashes.spec.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* chore: address review comments

* Changes from lint:fix

* chore: add review changes

* chore: address review comments

* chore:address review comments

* chore: address review comments

* chore: release note fix

---------

Co-authored-by: cloud-sdk-js <[email protected]>
Co-authored-by: Junjie Tang <[email protected]>
Co-authored-by: Deeksha Sinha <[email protected]>
KavithaSiva added a commit that referenced this pull request Dec 23, 2024
… - Option 2 (#5247)

* chore: append basePath to path pattern in openapi generator

* chore: switch to using removeSlashes from util

* chore: add additional test

* chore: check-public-api change

* Changes from lint:fix

* chore: update test

* chore: adjust all tests

* chore: fix codeQL warning

* Changes from lint:fix

* chore: add changeset

* chore: cleanup tests

* feat: extend openapi request builder for basePath

* chore: introduce setBasePath

* feat: introduce _defaultBasePath to the generated files

* chore: fix order of parameters with intro of basepath

* chore: fix tests

* chore: add setBasePath

* chore: fix failing e2e test

* Changes from lint:fix

* chore: cleanup path

* Changes from lint:fix

* chore: adjust test

* Changes from lint:fix

* chore: switch to using undefined for default basePath value

* chore: cleanup test

* chore: update changeset

* chore: adjust changeset

* chore: fix unit test

* chore:cleanup

* chore: fix test

* chore: cleanup changesets

* fix: integration test

* chore: fix path

* chore:fix path

* chore: remove dead code

* chore: review comment

* Update .changeset/chilled-flowers-hide.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/openapi/src/openapi-request-builder.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* chore: address review comments

* chore: revert update to removeSlashes utility

* Changes from lint:fix

* chore: address review comments

* chore: tests fix

* chore: adjust tests

* chore: address review comments

* Changes from lint:fix

* fix: begin all paths using '/'

* chore: address review comments

* Update .changeset/chilled-flowers-hide.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update .changeset/chilled-flowers-freeze.md

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/util/src/remove-slashes.spec.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* Update packages/util/src/remove-slashes.spec.ts

Co-authored-by: Deeksha Sinha <[email protected]>

* chore: address review comments

* Changes from lint:fix

* chore: add review changes

* chore: address review comments

* chore:address review comments

* chore: address review comments

* chore: release note fix

---------

Co-authored-by: cloud-sdk-js <[email protected]>
Co-authored-by: Junjie Tang <[email protected]>
Co-authored-by: Deeksha Sinha <[email protected]>
KavithaSiva added a commit that referenced this pull request Dec 27, 2024
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.

4 participants