PR to support Model enhancements #2056

dilipkrish · 2017-10-01T16:07:19Z

#Model enhancements include

Support for JsonView

Part of a different effort (these are different concepts) edited `2017-10-07 10:51:59`

Support for spring data rest projects
Support for inheritance
Possibly support all of these in the light of OAS 3.0

(2022)

dilipkrish

I made it a PR so we can add comments etc. I added some initial thoughts. I feel like the plugin still needs some tweaking. Let me know what you think?

dilipkrish · 2017-10-01T16:09:15Z

springfox-schema/src/main/java/springfox/documentation/schema/ModelProjectionExtractor.java

+
+  public Optional<ResolvedType> extractProjection(ResolvedType type, List<Annotation> annotations, DocumentationType documentationType) {
+    ModelProjectionProviderPlugin selected =
+            modelProjectionProviders.getPluginFor(documentationType, new JacksonJsonViewProjectionProvider());


I think if there are plugin extensibility's we should use all of them not just the first one. That way we can layer information via plugins.

dilipkrish · 2017-10-01T16:14:10Z

...x-schema/src/main/java/springfox/documentation/schema/JacksonJsonViewProjectionProvider.java

+  }
+
+  @Override
+  public Optional<Class<?>> projectionFor(ResolvedType type, List<Annotation> typeAnnotations) {


This should return a ResolvedType IMO.

Also have you thought about how we might influence a projection not using annotations (I don't know how gson handles this)? I think it might be a good idea to extract a parameter object i.e. projection context that gives it access to/or parts of the RequestMappingContext

Oh never mind... I see what this is doing. I don't think this interface is accurate. I'm thinking of the return value as an alternate type for the type. This makes this interface is jackson specific and will not work for spring-data-rest I think.

I was thinking more in these terms. Lets take this example from here

public class Views { public static class Public { } public static class Internal extends Public { } } public class Item { @JsonView(Views.Public.class) public int id; @JsonView(Views.Public.class) public String itemName; @JsonView(Views.Internal.class) public String ownerName; }

I would think that this interface would behave something like this.

//Pseudo code on how this should be used. ProjectionContext context = new ProjectionContext( type, //Type to find a projection type annotations, //Annotations to help finding projections ... more params as needed ); JacksonJsonViewProjectionProvider plugin = createInstanceOfPlugin(); Optional<ResolvedType> projection = instance.projectionFor(context); //OR Optional<Model> projection = instance.projectionFor(context);

Here projection would be a either a in-memory ResolvedType of type ItemPublicView built using AlternateTypeBuilder or straight away building the Model object via the ModelBuilder, that has the public properties as we do here. What do you think?

As i know, Gson does not have full projection support. Just only SerializationStrategy to determine which properties will be serialized.

There is a method Optional <Class<?> getRequiredAnnotation() that will handle case with JsonView, and will be ignored in case of Spring Data Rest

And we might have a case, where there won't be any projection, just all Class... For example, no defined @Projection classes, or no @JSONVIEW annotation on field.

Dilip, that is outdated code....

@Override public List<ResolvedType> projectionsFor(ResolvedType type, Optional<? extends Annotation> requiredAnnotation) { List<Class<?>> projections = newArrayList(); if (requiredAnnotation.isPresent() && requiredAnnotation.get() instanceof JsonView) { projections = newArrayList(((JsonView)requiredAnnotation.get()).value()); } return from(projections).transform(toResolvedType()).toList(); }

In last commit I changed interface and implementation

I agreed, ProjectionContext is a good idea, to consolidate all required information to determine projection

But, for me Projection is ONLY a part of ModelContext, just additional information, to determine which properties must be serialized or deserialized.

dilipkrish · 2017-10-01T16:38:29Z

...ox-spi/src/main/java/springfox/documentation/spi/service/contexts/RequestMappingContext.java

@@ -187,6 +187,10 @@ public String getGroupName() {
  public <T extends Annotation> Optional<T> findControllerAnnotation(Class<T> annotation) {
    return handler.findControllerAnnotation(annotation);
  }
+
+  public List<Annotation> getAnnotations() {


Does the findControllerAnnotation findAnnotations not work? I feel like a plugin should request only annotations it knows about. This is very open interface in my opinion.

I've deleted that already. The main idea is to make common interface to handle all cases with projections.

dilipkrish · 2017-10-01T16:39:22Z

springfox-spring-web/src/main/java/springfox/documentation/spring/web/WebMvcRequestHandler.java

@@ -114,6 +116,11 @@ public String getName() {
  }

  @Override
+  public List<Annotation> getAnnotations() {


Same here... I feel like a plugin should request only annotations it knows about. This is very open interface in my opinion.

Agreed, last commit restored that.

dilipkrish · 2017-10-01T16:39:35Z

...fox-spring-web/src/main/java/springfox/documentation/spring/web/readers/operation/.gitignore

@@ -0,0 +1 @@
+/OperationResponseClassReader.java


Something wrong, I accidentally ignored.... I will fix it

dilipkrish · 2017-10-01T16:49:20Z

springfox-spi/src/main/java/springfox/documentation/spi/service/ProjectionProviderPlugin.java

+
+import springfox.documentation.spi.DocumentationType;
+
+public interface ProjectionProviderPlugin extends Plugin<DocumentationType> {


I feel like this plugin shouldn have something other thanDocumentationType as the discriminator. It should be something like ProjectionType which can have one of the following types

Jackson

Spring-Data-Rest

Something we haven't thought off.

Then we can select the right plugin rather than pick the first one the pluginManager finds.Thoughts?

For that we should have some explicit configuration. Any way we won't have all plugins at one time. So, JacksonJsonViewPlugin has the lowest priority. If Spring-Data-Rest is included in classpath it will overwrites with its own plugin with higher priority. And custom plugin has the highest priority will overwrite all plugins.

ProjectionType - will be auto configurable. And Spring-Data-Rest will rewrite it with it's name. But I think it is better to overwrite all plugin, and use priority then to use the configuration plugin name.

dilipkrish · 2017-10-01T16:50:09Z

springfox-spi/src/main/java/springfox/documentation/spi/service/ProjectionProviderPlugin.java

+   * @param annotation - projections of type 
+   * @return resolved projection names
+   */
+  boolean applyProjection(ResolvedType activeProjection, ResolvedType typeToApply, List<ResolvedType> typeProjections);


I did not understand this method. I'll bring up the code and take a look at it again.

@Override public boolean applyProjection(ResolvedType activeProjection, ResolvedType typeToApply, Optional<? extends Annotation> requiredAnnotation) { final Class<?> activeView = activeProjection.getErasedType(); if (requiredAnnotation.isPresent() && requiredAnnotation.get() instanceof JsonView && activeView != null) { final Class<?>[] typeProjections = ((JsonView)requiredAnnotation.get()).value(); int i = 0, len = typeProjections.length; for (; i < len; ++i) { if (typeProjections[i].isAssignableFrom(activeView)) { return true; } } } return false; }

Determine if class field belongs to given projection or not

PrabhuKandasamy · 2019-01-08T08:49:50Z

Hi, @PrabhuKandasamy.
I'm going to push a new algorithm that can handle cyclic references in a few weeks.

Ok. Waiting for that.

vangogh-ken · 2019-01-10T01:30:10Z

wait for this fix

phirzel · 2019-02-05T14:46:25Z

Hey, we would highly appreciate such an improvement as fast as possible.

(Currently we have to deploy each Service-Model resulting in a name clash on separate instances (for e.g. breaking changes, Openshift-Projects), which is not what we really want for operational reasons.)

MaksimOrlov · 2019-02-05T15:27:48Z

@phirzel
Almost done it. Going to clean little bugs on the next week and push the last changes.

abdulhafidz · 2019-02-19T19:34:55Z

what happened

MaksimOrlov · 2019-03-24T00:47:03Z

I've pushed changes. Still a few questions remains opened.

dilipkrish · 2019-05-27T22:27:59Z

@MaksimOrlov thank you! thank you! thank you! for working on this! 🙇

For all of you waiting for this feature, please provide us feedback.

joao-rebelo · 2019-05-29T17:59:03Z

Hi,

Was having weird bugs since yesterday, untill I realized the snapshot version has changed! (the good and bad of working with snapshots)
For what I was seeing, it was duplicating my models for each usage in diferent methods.
So 3 methods returning ObjectA and I would get model ObjectA, ObjectA_1, ObjectA_2.

I wasn't able to easily make a simple project to exemplify this.. but if you need I can try.

Thanks for new developments either way!

Regards

MaksimOrlov · 2019-05-29T18:58:06Z

@joao-rebel, thanks for the reporting the issue.
Could you, please, provide code listing for the ObjectA and for the controllers(methods) that use that class?

joao-rebelo · 2019-05-30T09:34:49Z

Hi,

Follows the snippet of my code:

RestController:

@RestController
@RequestMapping(path = "/basicdata/translation")
@Api(tags = "Translations", description = "Responsible for handling Translations", authorizations = {
		@Authorization("Bearer") })
public class TranslationRestController {

	@Autowired
	private TranslationService translationService;

	@ApiOperation(value = "Bulk get translations", notes = "Bulk returns a set of translations and returns a maps of Xentis IDs to a resolved multi-language translations")
	@GetMapping(path = "/bulk")
	@Nonnull
	public Mono<Map<HexId, MultiLangTranslation>> bulkGet(
			@ApiParam(value = "Ids of translations to fetch", required = true, allowMultiple = true) @RequestParam(name = "id", required = true) @Nonnull Set<HexId> theIds,
			@ApiParam(value = "Types of translations to get", required = true, allowMultiple = true) @RequestParam(name = "type", required = true) @Nonnull Set<TranslationType> theTypes) {
		return translationService.bulkGet(theIds, theTypes);
	}

	@ApiOperation(value = "Get all translations", notes = "Gets all the translations of the specified types and returns a maps of Xentis IDs to a resolved multi-language translations")
	@GetMapping(path = "")
	@Nonnull
	public Mono<Map<HexId, MultiLangTranslation>> getAll(
			@ApiParam(value = "Types of translations to get", required = true, allowMultiple = true) @RequestParam(name = "type", required = true) @Nonnull Set<TranslationType> theTypes) {
		return translationService.getAll(theTypes);
	}

	@ApiOperation(value = "Get all translations in a range", notes = "Gets all the translations within a specific range and returns a maps of Xentis IDs to a resolved multi-language translations")
	@GetMapping(path = "/range/{id}")
	@Nonnull
	public Mono<Map<HexId, MultiLangTranslation>> getRange(
			@ApiParam(value = "Sample Id within the provided range to fetch", required = true) @PathVariable(name = "id") @Nonnull HexId theId,
			@ApiParam(value = "Type of Xentis IDs Range to fetch translations for", required = true) @RequestParam(name = "range") @Nonnull RangeType theRange,
			@ApiParam(value = "Types of translations to get", required = true, allowMultiple = true) @RequestParam(name = "type", required = true) @Nonnull Set<TranslationType> theTypes) {
		return translationService.getRange(theId, theRange, theTypes);
	}
}

Docklet:

@Configuration
@EnableSwagger2WebFlux
public class SwaggerConfig {

	@Autowired
	private TypeResolver resolver;

	@Bean
	public Docket api() {
		return new Docket(DocumentationType.SWAGGER_2) //
				.ignoredParameterTypes(ServerWebExchange.class)
				.select()
				.apis(RequestHandlerSelectors.any())
				.paths(PathSelectors.any())
				.build()
				.apiInfo(apiEndPointsInfo())
				.groupName("Basic Data")
				.directModelSubstitute(HexId.class, HexIdModel.class)
				.alternateTypeRules(
						new RecursiveAlternateTypeRule(
								resolver,
								Arrays.asList(
										AlternateTypeRules.newRule(
												resolver.resolve(Mono.class, WildcardType.class),
												resolver.resolve(WildcardType.class)),
										AlternateTypeRules.newRule(
												resolver.resolve(ResponseEntity.class, WildcardType.class),
												resolver.resolve(WildcardType.class)))))
				.alternateTypeRules(
						new RecursiveAlternateTypeRule(
								resolver,
								Arrays.asList(
										AlternateTypeRules.newRule(
												resolver.resolve(Flux.class, WildcardType.class),
												resolver.resolve(List.class, WildcardType.class)),
										AlternateTypeRules.newRule(
												resolver.resolve(ResponseEntity.class, WildcardType.class),
												resolver.resolve(WildcardType.class)))));
	}

	private ApiInfo apiEndPointsInfo() {
		return new ApiInfoBuilder().title("Data API")
				.version("0.0.1")
				.build();

	}

	//For Swagger Editor compatibility.Check:https://github.com/swagger-api/swagger-core/issues/2944
	@Bean
	public JacksonModuleRegistrar swaggerJacksonModuleRegistrar() {
		return objectMapper -> ReferenceSerializationConfigurer.serializeAsComputedRef(objectMapper);
	}

the ObjectA, aka the MultiLangTranslation:

public class MultiLangTranslation extends HashMap<String, Translation> {

	/**
	 * The long <code>serialVersionUID</code>.
	 */
	private static final long serialVersionUID = 1360228790913298738L;
	//Marker class
}

The Translation:

public class Translation implements Serializable {

	/**
	 * The long <code>serialVersionUID</code>.
	 */
	private static final long serialVersionUID = -26163810284100442L;

	private String small;
	private String medium;
	private String big;
        .......

and the HexId:

public class HexId implements Serializable {

	private static final long serialVersionUID = 201711281450L;

	private long id;

	public HexId(String theHexValue) {
		this(Long.parseLong(theHexValue, 16));
	}

	public HexId(long theId) {
		id = theId;
	}

	public String getValue() {
		return Long.toHexString(id).toUpperCase();
	}

	public Long getId() {
		return id;
	}

Since my Service had 3 methods I was getting the Translation and the MultiLangTranslation repeated 3 times.
During my debug I was finding out that at a method (not finding it now in the old snapshot) which tries to find the root models from the Mono<Map<HexId, MultiLangTranslation>> it was only returning the HexId object, since the MultiLangTranslation extended from Map.

But! This also happened for other services where the principal objects didn't extended from Map, but instead referenced directly or indirectly this MultiLangTranslation.

Also for another service where the returns were only: Mono<Map<HexId, List>> is only generated one HexId model, but it didn't allow to make the directModelSubstitute(HexId.class, HexIdModel.class), as it would throw an exception saying that HexId would have been already registered.

Sory for the huge information, hope it can help..

MaksimOrlov · 2019-05-30T10:03:11Z

@joao-rebelo thanks a lot for this infromation, Could you please add the generated json schema file. And could you, please, provide full code listing for Translation class with getters and setter. As I can see class HexId is not symmetric (difference between getters and setters). So for serialization and deserialization will be generated two different models.

joao-rebelo · 2019-05-30T13:34:51Z

Follows the translation. I believe it is symmetric.

public class Translation implements Serializable {

	/**
	 * The long <code>serialVersionUID</code>.
	 */
	private static final long serialVersionUID = -26163810284100442L;

	private String small;
	private String medium;
	private String big;

	public Translation() {
		//FOR SPRING DESERIALIZATION ONLY
	}

	public Translation(String theMedium) {
		small = null;
		medium = theMedium;
		big = null;
	}

	public Translation(String theSmall, String theMedium, String theBig) {
		small = theSmall;
		medium = theMedium;
		big = theBig;
	}

	public String getSmall() {
		return small;
	}

	public String getMedium() {
		return medium;
	}

	public String getBig() {
		return big;
	}

	@Override
	public String toString() {
		return new StringBuilder("{SMALL=").append(small).append(", MEDIUM=").append(medium).append(", BIG=").append(big).append("}").toString();
	}
}

The generated doc:

api-docs.txt

MaksimOrlov · 2019-05-30T16:32:51Z

Ok.
@joao-rebelo, I will make a test based on your listing and then fix everything, that looks like ambiguous at the swagger output file. Will back to you in a week.

dilipkrish and others added 10 commits September 13, 2017 21:38

Upgrading the dependencies before starting on work

63f4586

(2022)

Added new module to support oas 3.0

c1dae73

(2022)

Created an empty shell based on swagger 2.0

30c08e6

(2022)

Added mapping support for servers and variables

3aa9e0d

(2022)

Added mapping support for external docs

b566a48

(2022)

WIP

fa0edd8

Initial support for projections.

88375aa

Improvements for projections plugin.

8f67cc5

Added projection support for ModelAttribute parameters.

76a726e

Added algorithm to compare model names.

270aed0

dilipkrish commented Oct 1, 2017

View reviewed changes

Fixed wrong .gitIgnore

2961f20

springfox deleted a comment Oct 1, 2017

MaksimOrlov added 2 commits October 13, 2017 20:58

Optimization of ProjectionProviderPlugin.

e498db9

Renamed projection into a view.

59e9b8f

springfox deleted a comment Oct 13, 2017

MaksimOrlov added 3 commits March 24, 2019 02:36

New merging algorithm that can handle cyclic model references.

273b8d2

Unit tests.

2c4a234

Contract tests.

79a97fd

MaksimOrlov added 9 commits March 28, 2019 22:26

Fix bug with duplicates subTypes.

7bcaa63

Fix for redundant circles in known models.

1be9618

Ported circleci 2.0 config from master.

6818f6e

Additional test for recursive types.

250f44b

Fix for #2407.

7484904

Fix check style.

40534a0

Fix for endless recursion #2407.

9cdc29a

Removed extra check.

510eb4c

Optimization.

40a71a6

dilipkrish merged commit 40a71a6 into master May 27, 2019

dilipkrish deleted the feature/model-enhancements branch May 27, 2019 22:27

dilipkrish removed the in progress label May 27, 2019

dilipkrish added this to the 3.0 milestone May 27, 2019

dilipkrish mentioned this pull request Jul 14, 2020

Announcement: 3.0.0 Release #3386

Closed


		import springfox.documentation.spi.DocumentationType;

		public interface ProjectionProviderPlugin extends Plugin<DocumentationType> {

		@@ -0,0 +1 @@
		/OperationResponseClassReader.java

PR to support Model enhancements #2056

PR to support Model enhancements #2056

Conversation

dilipkrish commented Oct 1, 2017 • edited by MaksimOrlov

Part of a different effort (these are different concepts) edited 2017-10-07 10:51:59

dilipkrish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaksimOrlov Oct 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaksimOrlov Oct 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaksimOrlov Oct 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrabhuKandasamy commented Jan 8, 2019

vangogh-ken commented Jan 10, 2019

phirzel commented Feb 5, 2019

MaksimOrlov commented Feb 5, 2019

abdulhafidz commented Feb 19, 2019

MaksimOrlov commented Mar 24, 2019

dilipkrish commented May 27, 2019

joao-rebelo commented May 29, 2019

MaksimOrlov commented May 29, 2019 • edited

joao-rebelo commented May 30, 2019 • edited

MaksimOrlov commented May 30, 2019 • edited

joao-rebelo commented May 30, 2019

MaksimOrlov commented May 30, 2019 • edited

dilipkrish commented Oct 1, 2017 •

edited by MaksimOrlov

Part of a different effort (these are different concepts) edited `2017-10-07 10:51:59`

MaksimOrlov Oct 1, 2017 •

edited

MaksimOrlov Oct 1, 2017 •

edited

MaksimOrlov Oct 1, 2017 •

edited

MaksimOrlov commented May 29, 2019 •

edited

joao-rebelo commented May 30, 2019 •

edited

MaksimOrlov commented May 30, 2019 •

edited

MaksimOrlov commented May 30, 2019 •

edited