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

[jimshowalter]

As reported in https://github.com/swagger-api/swagger-parser/issues/1081 and https://github.com/swagger-api/swagger-parser/issues/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]

@jimshowalter which version of the parser do you use?

[jimshowalter]

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]

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]

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]

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]

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

[jimshowalter]

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

[gracekarina]

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]

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]

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]

Did the simplified example help?

[gracekarina]

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]

Hi @jimshowalter https://github.com/swagger-api/swagger-parser/pull/1559 here I made some changes, and seems like there are not duplication, can you please check? thanks

[jimshowalter]

I can try to use this branch locally tomorrow.

[jimshowalter]

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]

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]

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]

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]

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]

"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]

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

[jimshowalter]

Correct.

[jimshowalter]

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]

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

[jimshowalter]

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]

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]

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]

This shows the infinite recurse:

example0.zip

[jimshowalter]

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

[jimshowalter]

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]

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]

Have you been able to reproduce the problems?

[gracekarina]

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]

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

[gracekarina]

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]

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]

Hello, any updates on this?

[AmateurECE]

I believe I'm experiencing this issue at 08a76302e2501b3465b4febdafca733262a802d7 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]

Any updates?