Duplicate class names in different packages get squashed in ControllerDocumentation - modelMap

[mbazos]

I haven't yet made a fix for this yet but our team encountered this issue and I believe it's fairly easy to fix but wanted to get feedback from you on how to go about this.

Example of the issue given the following controller method:

    public void sampleMethod3(com.mangofactory.swagger.spring.sample.duplicate.Pet petOne, com.mangofactory.swagger.spring.sample.Pet petTwo) {
    }

Notice that petOne and petTwo are different classes but have the same class name. When the ControllerDocumnetation modelMap is being populated it uses a friendly class name as the key so it would be "Pet" in this instance. With "Pet" being the same key in the model map they will overwrite each other.

Ways I was thinking to go about fixing this:

1. Just add in the package name...so friendly names will now be fully qualified (Not so friendly names)
2. Add configuration into SwaggerConfiguration that would trigger friendly/fully qualified names

I am really not sure which one to do...friendly names are nice but people might end up not realizing this is happening so it's almost safer to have fully qualified.

Not sure if you need a reason of why this is important but here is it:

1. Service versioning - having multiple version of client model object could cause these collision to happen over time
2. Services that return subsets of a canonical model - some data might be expensive or unnecessary to return in this case service A could return petOne (superset) and service B could return petTwo (subset of petOne).

Let me know what you think and if I have time I don't mind working on this fix as it will benefit our team.

[dilipkrish]

@mbazos Interesting bug and totally an overlooked case :) Thanks for pointing it out! 👍. Couple of observations, not to be pedantic and all :-o!

• When you're thinking about these service interfaces, its almost a good idea not to leak internal implementation to the projected service interface.
• Two versions of a domain shouldn't co-mingle at service boundaries, that adaptation should again be an internal implementation.

With that out of the way, there is certainly a case that has the notion of the model namespaced by package rather than by the name of the type, like in your case com.xxxxx.canonical.Pet and com.xxxxx.summary.Pet.

I would say the simplest solution, (for now) rather than bringing in the package name etc. is the create a custom annotation.

public void sampleMethod3(@ModelAlias("PetCanonical") com.xxxxxx.canonical.Pet petOne, 
                                        @ModelAlias("PetSummary") com.xxxxxx.summary.Pet petTwo) {
    }

I'd modify ResolvedTypes to handle aliasing model names.

I think its fairly simple and also doesnt clutter the names with package names etc. I'm trying to get an intermediate release with 1.2 spec support, so, If you have some cycles I'd love to take in a pr! I'm planning to pay down some tech debt in 0.7 and modularizing the library. I could create a chore to make that an extensibility point.

Let me know if you have any questions.

[mbazos]

Thanks for getting back to me so quickly with this. So I completely agree with you on the two points you have outlined. Based on what you are saying and it completely makes sense is that I would rename my objects that are similar to give them a more distinct name.

For the example you outlined above you could have com.xxxxxx.canonical.Pet and com.xxxxxx.summary.PetSummary or com.xxxxxx.canonical.PetSummary. That way the domain objects are distinct and there is no confusion about what Pet domain object is what.

The above annotation becomes valuable when you supporting multiple versions of a given service (happens all the time because consumers might not want to move to the latest...for whatever reason). Imagine this package structure:

 com.xxxxxx.v1.canonical.Pet
 com.xxxxxx.v2.canonical.Pet

I know ideally your canonical model shouldn't change but in practice this happens I see these things happen:

1. Business needs/rules change
2. You learn there is a better way to represent your data for your clients
3. Deprecate features that are no longer applicable
4. Lazy consumers don't have "time" to upgrade

Service versioning is always an interesting topic. Again I really appreciate the help.

[donovanmuller]

I have a similar situation except that this is happening on the response class as well.
We have some generated JAXB classes which unfortunately share the same top level class. Even though the classes are fully qualified, springfox seems to pick one and use that everywhere.

Example below:

@ApiOperation(value = "Save something")
public blah.blah.Document create(@RequestBody foo.bar.Document thingy) {
  ...
  return new foo.bar.Document();
}

The Swagger docs will only contain one representation of Document...

[dilipkrish]

Yeah this needs to be fixed.

[stephenhibbert]

I'm also having the same issue. A fix would be very much appreciated!

[mbazos]

If I have some time I might work on this. I recently upgraded our apps from swagger-springmvc to springfox and I am really liking the re-organization and re-packaging of the libraries.

I am going to be out of the country and won't be back until the last week in September, I will make sure to check here before I start any work on this.

[dilipkrish]

@mbazos 🤘 would be much appreciated!

However, since this feature affects the core, I would like to collaborate on how you plan to approach this before you work on the PR. Just want to make sure its in-line with the direction we want to take this library.

Let me know if I can answer any questions.

[nikitsenka]

Could you please provide some aprox estimations when the issue will be fixed?

[dilipkrish]

ping @mbazos!

[mbazos]

Yes been very busy with life and work. Currently on break for the holidays will try to get this done before the new year.

[dilipkrish]

Not a problem... was just pinging to see if you still had plans to help with this feature. It will be much appreciated ... Thanks!

[mbazos]

@dilipkrish It's been a while since I have looked at this code and our discussion above references the old version of swagger-springmvc before you worked on the major refactor. Anyway I forked the repo and got my local workstation all up and running.

Just wanted to check a few things:
So I think what needs to change is DefaultTypeNameProvider, is this correct? Right now it's returning the class simple name which will conflict if more than one class have the same name.

Also I took a second look and I can see in the DefaultModelProvider the following:

  private Optional<Model> mapModel(ModelContext parentContext, ResolvedType resolvedType) {
    if (isMapType(resolvedType) && !parentContext.hasSeenBefore(resolvedType)) {
      String typeName = typeNameExtractor.typeName(parentContext);
      return Optional.of(parentContext.getBuilder()
          .id(typeName)
          .type(resolvedType)
          .name(typeName)
          .qualifiedType(ResolvedTypes.simpleQualifiedTypeName(resolvedType))
          .properties(new HashMap<String, ModelProperty>())
          .description("")
          .baseModel("")
          .discriminator("")
          .subTypes(new ArrayList<String>())
          .build());
    }
    return Optional.absent();
  }

I believe .id() needs to be the fully qualified name but .name() needs to be the simple name. Can you just let me know to make sure I am on the right path.

[dilipkrish]

I believe thats sounds correct but haven't dug in recently to tell u without spelunking. Off hand I know that there is also TypeNameExtractor that uses the TypeNameProviders plugins.

I thought I'd just chime in with a few expectations.

• It would be good to verify that your solution will work with swagger-codegen
• I would feature toggle this feature with a flag in the docket similar to the enableUrlTemplating flag _and mark with an @Incubating annotation_.
• Implement it in such a way that it falls back to current behavior if it doesn't find two models with the same name; similar to how we do operation unique ids using some kind of caching technique like here

I'll report back if I find anything different or interesting. Thanks for helping out!

[mbazos]

@dilipkrish just getting comfortable with the project and I have to be honest, I am trying to setup the things you mentioned above and I am partially there but I am having a hard time understanding how everything is connected.

I think I know the code changes I need to make in DefaultModelProvider but I am having a hard time understanding what I need to do to setup the plugin so I can make the decision of when to use this new behavior or not. I am a little confused about the operation/unique ids examples you sent me.

is there an irc chat or something that some of the devs use?

[duke-cliff]

@dilipkrish Why the ticket is marked as solved? What's the solution to use full name/package name instead of simple name for the model?

[duke-cliff]

Sorry, saw it was marked as solved in the release notes but not in the actual ticket

[MaksimOrlov]

@robertsearle, @benjvoigt , the fix has been merged to master branch. Could you, please, provide a feedback for 3.0.0-SNAPSHOT, if the issue was fixed or not ?

[benjvoigt]

@MaksimOrlov numbering appears to be stable between runs now, ty. I tested with multiple endpoints and also with nested models as return value.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed because it has not had recent activity. Please re-open a new issue if this is still an issue.

[ande13]

For java, you can use the following workaround

@primary
@component
public class SwaggerApiModelTypeNameProvider extends ApiModelTypeNameProvider {

private final Map<String, Pair<String, String>> classNames = new ConcurrentHashMap<>();

@Override
public String nameFor(Class<?> type) {
    ApiModel annotation = AnnotationUtils.findAnnotation(type, ApiModel.class);
    String defaultTypeName = type.getTypeName().replace(type.getPackageName() + ".", "");

    defaultTypeName = annotation != null ? Optional.ofNullable(Strings.emptyToNull(annotation.value())).orElse(defaultTypeName) : defaultTypeName;

    Pair<String, String> classPackagePair = classNames.get(defaultTypeName);
    if(classPackagePair != null) {
        if(!classPackagePair.getValue().equalsIgnoreCase(type.getPackageName())) {
            defaultTypeName = "_" + defaultTypeName;
        }
    }
    classNames.put(defaultTypeName, Pair.of(type.getTypeName(), type.getPackageName()));

    return defaultTypeName;

}

}

You can modify it if needed :)

[bestarshad]

please refer the files in repo for the fix

same class name issue fix in open api swagger it will generate schema request response as per java annotation naming convention against the java variables. it will not create schema request response as per java variables it will follow schema annotation

github

[xak2000]

@MaksimOrlov Just tried 3.0.0 and the issue is still not fixed.

Maybe because in my case the difference is not a package name, but an outer class name:

package my.package.dto;

public class User {
  private Location location;

  public static class Location {
    private String name;
  }
}

And:

package my.package.dto;

public class Car {
  private Location location;

  public static class Location {
    private Long id;
    private String address;
  }
}

Only one Location model is present in the generated docs. So, one of User or Car has incorrect model for location property.

[anibyl]

This helped me:

TypeNameResolver.std.setUseFqn(true);

[jyeonjyan]

@vignesharunachalam I had the same issue lately. Until a clear solution arises, you can always go around the problem by annotating at least one of your model with @ApiModel and use a unique name (e.g. LegacyPet for instance)

This solution solved my problem the fastest.
My situation for example..

- com.example.domain.dto.foo.Foo.Create
- com.example.domain.dto.bar.Bar.Create

package com.example.domain.dto.foo
@ApiModel("FooCreate")
data class Create(){}

package com.example.domain.dto.bar
@ApiModel("BarCreate")
data class Create(){}

[jyeonjyan]

@dilipkrish
First of all, thank you for your hard work.
So, is this issue resolved or won't fix?

[Tvaroh]

For newest swagger annotations this helps:

@Schema(name = "CustomName")

[yupnano]

ten years passed 😭