Remove servers urls with trailing slash #7940

ksvirkou-hubspot · 2020-11-14T14:52:02Z

Fixed issue #7533
I see it in every language

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.
If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. 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*. For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

ksvirkou-hubspot · 2020-11-14T16:55:49Z

Hi everyone
Could you take a look?
cc: @wing328 @spacether

ksvirkou-hubspot · 2020-11-23T12:07:05Z

Hi everyone
Could you take a look?
thank you in advance
cc: @wing328 @taxpon @frol @mbohlool @cbornet @kenjones-cisco @tomplus @Jyhess @arun-nalla @spacether @jimschubert @ackintosh @jmini @etherealjoy

atanasiuk-hubspot · 2020-11-26T09:49:50Z

Any updates on this?

yfaktor · 2020-11-27T14:07:31Z

Hi - this is holding back our major release, if you could be so kind to give this PR some love it would be greatly appreciated. Thanks!

spacether · 2020-11-27T19:22:02Z

Can you please add tests showing this code working for the use cases that you are concerned about?

ksvirkou-hubspot · 2020-12-24T10:40:53Z

I ve just added test
Could you check it ?

cc @wing328 @spacether

yfaktor · 2021-01-05T16:07:07Z

Hi - I was wondering if we could get a ship on this - we are pretty behind on the release and any help would be greatly appreciated. Thanks so much in advance!

spacether · 2021-01-05T18:12:32Z

Closing an re-opening to kick off CI tests.

spacether · 2021-01-06T00:01:45Z

@ksvirkou-hubspot there is a CI error here: Please run 'bin/utils/ensure-up-to-date' locally and commit changes (UNCOMMITTED CHANGES ERROR) Can you fix it? We can't merge it until all tests pass.

spacether · 2021-01-10T23:48:45Z

I updated the test to be in the java layer as this change impacts all generators

spacether · 2021-01-11T00:20:58Z

modules/openapi-generator/src/test/resources/3_0/issue_7533.yaml

+    url: https://www.apache.org/licenses/LICENSE-2.0.html
+  version: 1.0.0
+servers:
+  - url: /


Per @jimschubert
Server with no url base specified is meant to be the same server where the spec doc was downloaded from.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#server-object:
REQUIRED. A URL to the target host. This URL supports Server Variables and MAY be relative, to indicate that the host location is relative to the location where the OpenAPI document is being served. Variable substitutions will be made when a variable is named in {brackets}.
So we should only do this stripping if the length is > 1

Hmm when we keep this value as '/' rust tests fail because they assume that we keep stripping the / from the end of the server url. Probably better to keep removing it and have future PRs detect the "" use case if they need to. That way this is non-breaking.

spacether · 2021-01-15T22:34:05Z

Closing and re-opening for travis

ksvirkou-hubspot · 2021-01-20T08:22:49Z

Hey everyone
In my opinion it is really important fix for the latest version of the generator
We noticed this problem in ruby too
ping me if I can help you with it
CC @spacether

spacether · 2021-01-20T21:56:18Z

Closing and re-opening for travis

spacether · 2021-01-20T22:55:28Z

@ksvirkou-hubspot can you help get tests passing?
@wing328 I see this change in java jersey-2:

--- a/samples/openapi3/client/petstore/java/jersey2-java8-special-characters/src/main/java/org/openapitools/client/ApiClient.java
+++ b/samples/openapi3/client/petstore/java/jersey2-java8-special-characters/src/main/java/org/openapitools/client/ApiClient.java
@@ -72,7 +72,7 @@ public class ApiClient extends JavaTimeFormatter {
 
   protected List<ServerConfiguration> servers = new ArrayList<ServerConfiguration>(Arrays.asList(
     new ServerConfiguration(
-      "/",
+      "",
       "No description provided",
       new HashMap<String, ServerVariable>()
     )
Perform git status
On branch pull/7940
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)

Are you okay with it?
If no then

we will probably need to add a method that processes these server strings and have rust use it to get tests passing
or just update the rust expected test result to what our new code will do
See Remove servers urls with trailing slash #7940 (comment)

spacether · 2021-01-20T23:02:10Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java

@@ -1483,4 +1484,8 @@ private void generateFilesMetadata(List<File> files) {
        }
    }

+    private String removeTrailingSlash(String value) {


Maybe we should pass in a boolean doNotModifyOnlySlash and when it is true, for "/" leave it as-is?
That we we could use it to preserve the current behavior here for rust jersey-2 etc.

wing328 · 2021-01-25T03:01:05Z

cc @OpenAPITools/generator-core-team as there are changes to the default generator

ksvirkou-hubspot mentioned this pull request Nov 23, 2020

[BUG] [Python] it handles servers urls trailing slash incorrectly #7533

Closed

wing328 added Feature: Generator Issue: Bug labels Dec 8, 2020

wing328 added this to the 5.0.0 milestone Dec 8, 2020

wing328 modified the milestones: 5.0.0, 5.0.1 Dec 21, 2020

ksvirkou-hubspot and others added 6 commits January 10, 2021 15:47

remove trailing slash

b90f812

update sample app

94444eb

added tests

0c767a2

bug fix

a0e0a03

Adds test in DefaultGeneratorTest

d8d322a

Reverts python files

1fb0199

spacether reviewed Jan 11, 2021

View reviewed changes

spacether added 2 commits January 10, 2021 16:25

Does not modify a value of /

6be24b2

Stops skipping / use case

69ff970

spacether closed this Jan 15, 2021

spacether reopened this Jan 15, 2021

spacether closed this Jan 20, 2021

spacether reopened this Jan 20, 2021

spacether reviewed Jan 20, 2021

View reviewed changes

update samples

79f34dd

wing328 changed the title ~~it handles servers urls with trailing slash~~ Remove servers urls with trailing slash Jan 25, 2021

wing328 merged commit 57227e5 into OpenAPITools:master Jan 25, 2021

wing328 mentioned this pull request Jul 4, 2021

[Java] [Native] Remove trailing slashes from the baseUri #9837

Open

5 tasks

johnnyaug mentioned this pull request Jul 11, 2021

[Question] Should the generated Java client be sensitive to trailing slashes? #9931

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove servers urls with trailing slash #7940

Remove servers urls with trailing slash #7940

ksvirkou-hubspot commented Nov 14, 2020 •

edited

Loading

ksvirkou-hubspot commented Nov 14, 2020 •

edited

Loading

ksvirkou-hubspot commented Nov 23, 2020

atanasiuk-hubspot commented Nov 26, 2020

yfaktor commented Nov 27, 2020

spacether commented Nov 27, 2020

ksvirkou-hubspot commented Dec 24, 2020

yfaktor commented Jan 5, 2021

spacether commented Jan 5, 2021

spacether commented Jan 6, 2021 •

edited

Loading

spacether commented Jan 10, 2021

spacether Jan 11, 2021 •

edited

Loading

spacether Jan 12, 2021 •

edited

Loading

spacether commented Jan 15, 2021

ksvirkou-hubspot commented Jan 20, 2021 •

edited

Loading

spacether commented Jan 20, 2021

spacether commented Jan 20, 2021

spacether Jan 20, 2021 •

edited

Loading

wing328 commented Jan 25, 2021

Remove servers urls with trailing slash #7940

Remove servers urls with trailing slash #7940

Conversation

ksvirkou-hubspot commented Nov 14, 2020 • edited Loading

ksvirkou-hubspot commented Nov 14, 2020 • edited Loading

ksvirkou-hubspot commented Nov 23, 2020

atanasiuk-hubspot commented Nov 26, 2020

yfaktor commented Nov 27, 2020

spacether commented Nov 27, 2020

ksvirkou-hubspot commented Dec 24, 2020

yfaktor commented Jan 5, 2021

spacether commented Jan 5, 2021

spacether commented Jan 6, 2021 • edited Loading

spacether commented Jan 10, 2021

spacether Jan 11, 2021 • edited Loading

Choose a reason for hiding this comment

spacether Jan 12, 2021 • edited Loading

Choose a reason for hiding this comment

spacether commented Jan 15, 2021

ksvirkou-hubspot commented Jan 20, 2021 • edited Loading

spacether commented Jan 20, 2021

spacether commented Jan 20, 2021

spacether Jan 20, 2021 • edited Loading

Choose a reason for hiding this comment

wing328 commented Jan 25, 2021

ksvirkou-hubspot commented Nov 14, 2020 •

edited

Loading

ksvirkou-hubspot commented Nov 14, 2020 •

edited

Loading

spacether commented Jan 6, 2021 •

edited

Loading

spacether Jan 11, 2021 •

edited

Loading

spacether Jan 12, 2021 •

edited

Loading

ksvirkou-hubspot commented Jan 20, 2021 •

edited

Loading

spacether Jan 20, 2021 •

edited

Loading