External ref resolve fails to resolve to same schema and creates duplicate classes #1518

jimshowalter · 2021-01-18T21:23:51Z

As reported in #1081 and
#1422, something gets confused and creates Foo.java and Foo1.java, even though there is just one foo.json model.

Those issues report that the problem has been fixed, but it has not.

In https://github.com/jimshowalter/swagger-parser-external-ref-bug there is a schema that reproduces the problem.

Run the JavaJAXRSSpecServerCodegen.java code generator on it, and you'll see that it generates a Stunts.java and Stunts1.java, but there is only stunts.json.

Also in that repo is a fix, or at least a fix that works for us. Look for "// BEGIN REFERENCE FORK:".

webron · 2021-01-22T02:38:54Z

@jimshowalter which version of the parser do you use?

jimshowalter · 2021-01-22T19:53:59Z

implementation "io.swagger.codegen.v3:swagger-codegen-generators:${swaggerCodegenGeneratorsVersion}"
implementation "io.swagger.codegen.v3:swagger-codegen:${swaggerCodegenVersion}"
implementation "io.swagger.parser.v3:swagger-parser:${swaggerParserVersion}"
implementation "io.swagger.parser.v3:swagger-parser-v3:${swaggerParserVersion}"
implementation "io.swagger.parser.v3:swagger-parser-v2-converter:${swaggerParserVersion}"
implementation "io.swagger:swagger-parser:${swaggerParserVersion2}"
implementation "io.swagger.parser.v3:swagger-parser-core:${swaggerParserVersion}"
implementation "io.swagger.core.v3:swagger-models:${swaggerCoreVersion}"
implementation "io.swagger.core.v3:swagger-annotations:${swaggerCoreVersion}"
implementation "io.swagger.core.v3:swagger-core:${swaggerCoreVersion}"

swaggerCoreVersion=2.1.6
swaggerParserVersion=2.0.24
swaggerParserVersion2=1.0.54
swaggerCodegenVersion=3.0.24
swaggerCodegenGeneratorsVersion=1.0.24

jimshowalter · 2021-02-02T17:54:34Z

Were you able to reproduce the problem with those versions (which I think are current, or nearly so), and the repo I linked to?

gracekarina · 2021-02-05T15:31:25Z

Hi, may I ask why are you using two different kind of ref for the schemas?
in ones you use components schemas assuming it ir will be resolve and in the other you use the relative path to the file.

I also notice that in the main json file you don't use stunts, how ever is listed in the components->schemas section

      "oneOf": [
        {
          "$ref": "#/components/schemas/Rebato"
        },
        {
          "$ref": "#/components/schemas/Drabbet"
        },
        {
          "$ref": "#/components/schemas/Highvelds"
        },
        {
          "$ref": "#/components/schemas/Polly"
        }
      ],
      "description": "blah blah blah"
    },
    "tashotSipe": {
      "$ref": "../api.json#/components/schemas/TashotSipe"
    },
    "massage": {
      "type": "string",
      "description": "blah blah blah"
    },
    "stunts": {
      "$ref": "../api.json#/components/schemas/Stunts"
    }

jimshowalter · 2021-02-05T16:01:15Z

There's nothing in the OAS spec that makes this an invalid schema.

There's something in the implementation of the OAS parser that makes it unable to conclude that Stunts == Stunts.

If there's a way to rewrite the schema that will produce the same output (other than fixing the duplicates), please explain how to do that, and we can use that as a workaround.

gracekarina · 2021-02-05T16:32:36Z

Hi @jimshowalter , I will start working on a fix for this duplication issue. Haven't found a workaround so far.

jimshowalter · 2021-02-07T15:46:40Z

Thank you--the problem is in some of the most-complicated code in all of OAS!

gracekarina · 2021-02-07T22:02:51Z

Hi, @jimshowalter the only issue is that stunts_1 and TashotSipe_1 are being added? or is there an issue with the other schemas also being duplicated, as analemmata and reachless.

Is it possible for you to please create a simpler sample for me to work on, it would help the debugging, and also narrow down the real issue. thanks.

jimshowalter · 2021-02-08T15:37:19Z

I'll try to create a simpler version, but having already reduced the original real schema to what's in the example, another pair of duplicates vanished. I'm afraid whittling it down will unconfuse the parser and the last duplicates will disappear.

jimshowalter · 2021-02-13T20:50:21Z

Try now: https://github.com/jimshowalter/swagger-parser-external-ref-bug

I noticed something interesting after boiling it down to essentials. If you remove the enums from stunts.json and tashotSipe.json, then it generates Stunts1.java and TashotSipe1.java, but doesn't generate Stunts.java or TashotSipe.java.

The names are still wrong, because they shouldn't end in "1", but that's a clue.

jimshowalter · 2021-04-19T19:31:58Z

Did the simplified example help?

gracekarina · 2021-04-19T19:35:57Z

Hi, @jimshowalter I had to work on another issues, but now I'm back on this one. Yes, the simplified example helped and the enum comment gave me an idea. Thanks. I hope to have a solution soon.

gracekarina · 2021-05-06T17:24:36Z

Hi @jimshowalter #1559 here I made some changes, and seems like there are not duplication, can you please check? thanks

In case $ref refers to path with parameter, using whole $ref in processRefSchema as externalFile argument results in URISyntaxException java.net.URISyntaxException: Illegal character in fragment at index 49: ../v1/checkout.yml#/paths/~1checkout~1orderItem~1{orderItemId} at java.base/java.net.URI$Parser.fail(URI.java:2915) at java.base/java.net.URI$Parser.checkChars(URI.java:3086) at java.base/java.net.URI$Parser.parse(URI.java:3130) at java.base/java.net.URI.<init>(URI.java:600) at io.swagger.v3.parser.processors.ExternalRefProcessor.join(ExternalRefProcessor.java:977) at io.swagger.v3.parser.processors.ExternalRefProcessor.constructRef(ExternalRefProcessor.java:928) at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefSchema(ExternalRefProcessor.java:918) at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefSchemaObject(ExternalRefProcessor.java:867) at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalPathItem(ExternalRefProcessor.java:280) at io.swagger.v3.parser.processors.PathsProcessor.processReferencePath(PathsProcessor.java:299) at io.swagger.v3.parser.processors.PathsProcessor.processPaths(PathsProcessor.java:63) at io.swagger.v3.parser.OpenAPIResolver.resolve(OpenAPIResolver.java:49) Using only file part (without fragment) produces expected result. fixing 1518 test test for 1518 adding missing fils removing duplicated code

jimshowalter · 2021-05-07T02:17:28Z

I can try to use this branch locally tomorrow.

jimshowalter · 2021-05-08T20:51:29Z

I'm sorry to report that this code is modifying model and reference names.

I deleted our one forked class, dropped io.swagger.v3 into our code generator under src/main/java, and ran the regression tests.

The tests just compare what's generated to what's checked in (baselines).

Some example diffs:

-import com.abc.api.foo.v2.m4.model.DeviceInfo;
-import com.abc.api.foo.v2.m4.model.GroupInfo;
-import com.abc.api.foo.v2.m4.model.TermsOfServiceInfo;
-import com.abc.api.foo.v2.m4.model.UserInfo;
+import com.abc.api.foo.v2.m4.model.ApiJsoncomponentsschemasdeviceInfo;
+import com.abc.api.foo.v2.m4.model.ApiJsoncomponentsschemasgroupInfo;
+import com.abc.api.foo.v2.m4.model.ApiJsoncomponentsschemastermsOfServiceInfo;
+import com.abc.api.foo.v2.m4.model.ApiJsoncomponentsschemasuserInfo;

and:

-public Foo setBar(BarReference bar) {
+public Foo setBar(ApiJsoncomponentsschemasBarReference bar) {

and this particularly bad one:

import java.util.Objects;
-import com.abc.api.foo.v2.m4.model.PagedCollection;
-import com.abc.api.foo.v2.m4.model.Pagination;
-import com.abc.api.foo.v2.m4.model.Foo;
+import com.abc.api.foo.v2.m4.model.ApiJsoncomponentsschemasFoo;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonCreator;

I've seen these behaviors before, and it's our forked class that prevents them.

gracekarina · 2021-05-08T21:29:28Z

Hi @jimshowalter, I suggest you test first the spec you sent me it's getting the expected results and the schemas resolved are indeed the ones expected, what you sent up there is not cause by the code I sent, it could be something else. please check the repo you sent me with the changes I made. thanks
https://github.com/jimshowalter/swagger-parser-external-ref-bug

What I sent below is the yaml.prettyPrint() of components.schemas that the parser resolved, of the spec you share with us.

schemas:
  Analemmata:
    type: object
    properties:
      tashotSipe:
        $ref: '#/components/schemas/TashotSipe'
      stunts:
        $ref: '#/components/schemas/Stunts'
    description: blah blah blah
  TashotSipe:
    type: string
    description: blah blah blah
    enum:
    - RAFTER
  Stunts:
    type: string
    description: blah blah blah
    enum:
    - XORK
  Essot:
    type: object
    properties:
      statue:
        type: integer
        description: blah blah blah
        format: int32
    description: blah blah blah

jimshowalter · 2021-05-09T21:28:21Z

I understand that it works with the reduced schema I created to reproduce the problem.

I'm just saying that the fix introduces other problems (or that removing our fork introduces other problems).

jimshowalter · 2021-05-09T21:35:24Z

Here are the Swagger dependencies we're using:

implementation "io.swagger.codegen.v3:swagger-codegen-generators:${swaggerCodegenGeneratorsVersion}"
implementation "io.swagger.codegen.v3:swagger-codegen:${swaggerCodegenVersion}"
implementation "io.swagger.parser.v3:swagger-parser:${swaggerParserVersion}"
implementation "io.swagger.parser.v3:swagger-parser-v3:${swaggerParserVersion}"
implementation "io.swagger.parser.v3:swagger-parser-v2-converter:${swaggerParserVersion}"
implementation "io.swagger:swagger-parser:${swaggerParserVersion2}"
implementation "io.swagger.parser.v3:swagger-parser-core:${swaggerParserVersion}"
implementation "io.swagger.core.v3:swagger-models:${swaggerCoreVersion}"
implementation "io.swagger.core.v3:swagger-annotations:${swaggerCoreVersion}"
implementation "io.swagger.core.v3:swagger-core:${swaggerCoreVersion}"

swaggerCoreVersion=2.1.9
swaggerParserVersion=2.0.25
swaggerParserVersion2=1.0.54
swaggerCodegenVersion=3.0.25
swaggerCodegenGeneratorsVersion=1.0.25

gracekarina · 2021-05-10T20:55:49Z

As I mentioned in the last comment, I think we cannot mix issues here. If the issue of the duplication is fixed 🎉 , I don't think the changing model name issue, is related to this PR at all. Maybe you can test with current versions before I merged this PR. So you can be certain this PR has nothing to do with that. Ater that you can send us a spec where we can reproduce the issue of the changing models name, wich is probably related to another project, that creates the models names.

jimshowalter · 2021-05-11T18:10:12Z

"Maybe you can test with current versions". I'm already doing that. See the above list. Everything is on the latest. That's why I'm saying, the only thing that's change is the code in this branch. Our schemas haven't changed. So the model names can only be changing due to the code in this branch. It's the only variable. Everything else is current, and constant.

I'll try to put together another example that demonstrates this latest problem. But you can't merge this branch because it will break things, even if it fixes the original problem.

gracekarina · 2021-05-11T18:22:25Z

Hi!, you mean, you have tested with this branch and with master branch, and with master the changing model name did not happend?

jimshowalter · 2021-05-11T18:23:36Z

Correct.

jimshowalter · 2021-05-11T18:27:17Z

The way our codegen tests work is we have the above dependencies, and we run the ben-manes dependency analyzer (in every build) to see if there's anything new. If so, we update it. Then we run our tests, which are based on the models for > 40 microservices, and on baselines that are checked in. The baselines are from the previous run, after reviewing them for any diffs. What I'd hoped to see after deleting our one forked class and dropping your code into the parser repo (and locally publishing the jar) was zero diffs. Instead, we got the diffs outlined above. And the only thing that changed was the code from this branch. Occasionally when we upgrade one of the codegen dependencies we see very minor changes, but almost always the baselines compare exactly, even including formatting (because the templates we use don't change).

jimshowalter · 2021-05-11T18:27:34Z

I'm trying to create a cut-down version of one of our schemas that shows the problem.

jimshowalter · 2021-05-12T23:56:11Z

Haven't been able to cut down the schema yet, but here's one of the symptoms:

Exception in thread "Thread-5" java.lang.StackOverflowError
at io.swagger.v3.oas.models.media.Schema.toString(Schema.java)
at io.swagger.v3.oas.models.media.StringSchema.toString(StringSchema.java:84)
at java.lang.String.valueOf(String.java:2994)
at java.lang.StringBuilder.append(StringBuilder.java:131)
at java.util.AbstractMap.toString(AbstractMap.java:559)
at io.swagger.v3.oas.models.media.Schema.toIndentedString(Schema.java:894)
at io.swagger.v3.oas.models.media.Schema.toString(Schema.java:872)
at io.swagger.v3.oas.models.media.ObjectSchema.toString(ObjectSchema.java:70)
at java.lang.String.valueOf(String.java:2994)
at java.lang.StringBuilder.append(StringBuilder.java:131)
at io.swagger.v3.parser.processors.ExternalRefProcessor.finalNameRec(ExternalRefProcessor.java:75)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:111)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
at io.swagger.v3.parser.processors.ExternalRefProcessor.processRefToExternalSchema(ExternalRefProcessor.java:150)
etc.

gracekarina · 2021-05-13T00:12:42Z

Hi @jimshowalter, thanks, while you can send us a minimal spec exposing the issue, I have to move on to a new one. I'm looking forward to see the sample so this can be finally fixed. Thanks for this.

jimshowalter · 2021-05-13T00:39:27Z

Yes, that's fine. Working on the spec.

Looking at your branch, it appears that after ignoring tests, the schema, and changes to code that only affect formatting, there is only one file with actual changes: ExternalRefProcessor.java. Can you confirm that's correct?

jimshowalter · 2021-05-13T01:13:47Z

This shows the infinite recurse:

example0.zip

jimshowalter · 2021-05-13T01:14:07Z

I'll work on examples of the other problems tomorrow.

jimshowalter · 2021-05-13T17:52:29Z

This one outputs:

public Error setBad(List<CommonmodelsbadJson> bad) {

but it should output:

public Error setBad(List<Bad> bad) {

Note that config.json specifies a modelName.

example1.zip

jimshowalter · 2021-05-13T19:02:56Z

This one doesn't generate anything for porchOperation.json despite it being referred to in the API schema.

In addition, it generates:

public CollectionItemCreationProps setFoos(CollectionsApiJsoncomponentsschemasLink foos) {

instead of:

public CollectionItemCreationProps setFoos(Link foos) {

If you change the names of collectionCreationProps.json and collectionItemCreationProps.json at all, it makes it stop doing that.

example2.zip

jimshowalter · 2021-05-14T23:36:11Z

Have you been able to reproduce the problems?

gracekarina · 2021-06-03T22:29:53Z

Hi, @jimshowalter we have merged a solution for the duplication schema issue, and we also released parser, please let us know if this solves the issue.

jimshowalter · 2021-06-15T23:32:02Z

I tested it by deleting the forked ExternalRefProcessor, and it fails with a bunch of errors.

gracekarina · 2021-06-15T23:51:42Z

That was not the only class that changed, so please do a proper test. If you find other issues I'll apreciate if you create new issues and link it to this one.

jimshowalter · 2021-06-18T18:16:34Z

I did a comprehensive test!

We test our code generation by starting with 40+ schemas for our services, running the generator at time T0, and checking in the resulting generated code. Then whenever we make a change to our schemas, or to our templates, or to the versions of Swagger we use, or any other change, our unit tests rerun, regenerating everything from the schemas, and fail the build if there are any diffs (including spacing and formatting changes).

If the build fails, we review the diffs. If the diffs are expected, or unexpected but acceptable, we check in the latest generated code, which becomes our new baselines as of time T1. This cycle repeats forever whenever there is a change.

Whenever we launch a new service, we add its schemas and baseline to the test framework.

To test your change, I upgraded all of the Swagger versions, deleted ExternalRefProcessor, and got a bunch of diffs. The diffs are coming entirely from Swagger, because with ExternalRefProcessor no longer forked, we are using the very latest Swagger everything, which includes your parser changes.

The zips I attached show various ways in which the latest Swagger generates code that doesn't match our baselines, with ExternalRefProcessor no longer forked.

You should be able to easily reproduce the problems by unzipping the schemas and running the latest parser on them.

regniblod · 2023-05-25T10:34:05Z

Hello, any updates on this?

AmateurECE · 2023-08-01T02:08:55Z

I believe I'm experiencing this issue at 08a7630 with my OpenAPI 3.0.1 specification. Is there a chance I'm experiencing a regression, or has this not been fully resolved yet?

codepred · 2024-03-14T12:44:50Z

Any updates?

gracekarina self-assigned this Feb 5, 2021

gracekarina added a commit that referenced this issue May 6, 2021

possible fix for #1518

97d68ac

jimshowalter mentioned this issue May 26, 2021

Duplicate classes / Models are generated if ( $ref) referenced externally. OpenAPITools/openapi-generator#2701

Open

gracekarina mentioned this issue May 27, 2021

Fix for 1518 with no SOF #1566

Merged

AmateurECE mentioned this issue Aug 1, 2023

Duplicated referenced definitions #1422

Open

ctreatma linked a pull request Aug 9, 2024 that will close this issue

Normalize relative ref paths to avoid duplicating schemas #2105

Open

External ref resolve fails to resolve to same schema and creates duplicate classes #1518

External ref resolve fails to resolve to same schema and creates duplicate classes #1518

Comments

jimshowalter commented Jan 18, 2021

webron commented Jan 22, 2021

jimshowalter commented Jan 22, 2021 • edited Loading

jimshowalter commented Feb 2, 2021

gracekarina commented Feb 5, 2021 • edited Loading

jimshowalter commented Feb 5, 2021

gracekarina commented Feb 5, 2021

jimshowalter commented Feb 7, 2021

gracekarina commented Feb 7, 2021 • edited Loading

jimshowalter commented Feb 8, 2021

jimshowalter commented Feb 13, 2021

jimshowalter commented Apr 19, 2021

gracekarina commented Apr 19, 2021

gracekarina commented May 6, 2021

jimshowalter commented May 7, 2021

jimshowalter commented May 8, 2021 • edited Loading

gracekarina commented May 8, 2021 • edited Loading

jimshowalter commented May 9, 2021

jimshowalter commented May 9, 2021

gracekarina commented May 10, 2021

jimshowalter commented May 11, 2021

gracekarina commented May 11, 2021

jimshowalter commented May 11, 2021

jimshowalter commented May 11, 2021

jimshowalter commented May 11, 2021

jimshowalter commented May 12, 2021 • edited Loading

gracekarina commented May 13, 2021

jimshowalter commented May 13, 2021

jimshowalter commented May 13, 2021

jimshowalter commented May 13, 2021

jimshowalter commented May 13, 2021 • edited Loading

jimshowalter commented May 13, 2021 • edited Loading

jimshowalter commented May 14, 2021

gracekarina commented Jun 3, 2021

jimshowalter commented Jun 15, 2021

gracekarina commented Jun 15, 2021

jimshowalter commented Jun 18, 2021 • edited Loading

regniblod commented May 25, 2023

AmateurECE commented Aug 1, 2023

codepred commented Mar 14, 2024

jimshowalter commented Jan 22, 2021 •

edited

Loading

gracekarina commented Feb 5, 2021 •

edited

Loading

gracekarina commented Feb 7, 2021 •

edited

Loading

jimshowalter commented May 8, 2021 •

edited

Loading

gracekarina commented May 8, 2021 •

edited

Loading

jimshowalter commented May 12, 2021 •

edited

Loading

jimshowalter commented May 13, 2021 •

edited

Loading

jimshowalter commented May 13, 2021 •

edited

Loading

jimshowalter commented Jun 18, 2021 •

edited

Loading