Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use JsonValue method type when present. #449

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public void applyToConfigBuilder(SchemaGeneratorConfigBuilder builder) {
applySubtypeResolverToConfigBuilder(generalConfigPart, fieldConfigPart, methodConfigPart);

generalConfigPart.withCustomDefinitionProvider(new JsonUnwrappedDefinitionProvider());
generalConfigPart.withCustomDefinitionProvider(new JsonValueDefinitionProvider());
}

private void applySubtypeResolverToConfigBuilder(SchemaGeneratorGeneralConfigPart generalConfigPart,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright 2020 VicTools.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Time traveler 😉

Suggested change
* Copyright 2020 VicTools.
* Copyright 2024 VicTools.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.github.victools.jsonschema.module.jackson;

import com.fasterxml.classmate.ResolvedType;
import com.fasterxml.classmate.members.ResolvedMethod;
import com.fasterxml.jackson.annotation.JsonValue;
import com.github.victools.jsonschema.generator.CustomDefinition;
import com.github.victools.jsonschema.generator.CustomDefinitionProviderV2;
import com.github.victools.jsonschema.generator.SchemaGenerationContext;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Implementation of the {@link CustomDefinitionProviderV2} interface for treating object types based on a {@link JsonValue} annotation
* being present with {@code value = true} on exactly one argument-free method. If no such annotations exist, no custom definition will be returned;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Suggested change
* being present with {@code value = true} on exactly one argument-free method. If no such annotations exist, no custom definition will be returned;
* being present with {@code value = true} on exactly one argument-free method. If no such annotation exists, no custom definition will be returned;

* thereby falling back on whatever is defined in a following custom definition (e.g. from one of the standard generator {@code Option}s).
*/
public class JsonValueDefinitionProvider implements CustomDefinitionProviderV2 {

@Override
public CustomDefinition provideCustomSchemaDefinition(ResolvedType javaType, SchemaGenerationContext context) {
ResolvedMethod jsonValueAnnotatedMethod = getJsonValueAnnotatedMethod(javaType, context);
if (jsonValueAnnotatedMethod == null) {
return null;
}
return new CustomDefinition(context.createDefinition(jsonValueAnnotatedMethod.getType()));
Comment on lines +35 to +43
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: A custom definition provider by design should be the last resort when realizing some requirement.
In this case, I'd avoid it to not get into the messy realm of "when to inline and when to declare in the central $defs".
Fortunately, there is already an alternative for exactly this kind of "replacing one type with another in the schema": a SubtypeResolver (even if this is not necessarily an actual subtype here).

Suggested change
public class JsonValueDefinitionProvider implements CustomDefinitionProviderV2 {
@Override
public CustomDefinition provideCustomSchemaDefinition(ResolvedType javaType, SchemaGenerationContext context) {
ResolvedMethod jsonValueAnnotatedMethod = getJsonValueAnnotatedMethod(javaType, context);
if (jsonValueAnnotatedMethod == null) {
return null;
}
return new CustomDefinition(context.createDefinition(jsonValueAnnotatedMethod.getType()));
public class JsonValueTypeResolver implements SubtypeResolver {
@Override
public List<ResolvedType> findSubtypes(ResolvedType javaType, SchemaGenerationContext context) {
ResolvedMethod jsonValueAnnotatedMethod = this.getJsonValueAnnotatedMethod(javaType, context);
if (jsonValueAnnotatedMethod == null) {
return null;
}
return Arrays.asList(jsonValueAnnotatedMethod.getType());

}

/**
* Look-up the single {@link JsonValue} annotated method with {@code value = true} and no expected arguments.
*
* @param javaType targeted type to look-up serialization method for
* @param context generation context providing access to type resolution context
* @return single method with {@link JsonValue} annotation
*/
protected ResolvedMethod getJsonValueAnnotatedMethod(ResolvedType javaType, SchemaGenerationContext context) {
ResolvedMethod[] memberMethods = context.getTypeContext().resolveWithMembers(javaType).getMemberMethods();
Set<ResolvedMethod> jsonValueAnnotatedMethods = Stream.of(memberMethods)
.filter(method -> method.getArgumentCount() == 0)
.filter(method -> Optional.ofNullable(method.getAnnotations().get(JsonValue.class)).map(JsonValue::value).orElse(false))
.collect(Collectors.toSet());
if (jsonValueAnnotatedMethods.size() == 1) {
return jsonValueAnnotatedMethods.iterator().next();
}
return null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ static class TestClass {

public TestEnumWithJsonPropertyAnnotations enumValueWithJsonPropertyAnnotations;

public TestTypeWithJsonValue typeWithJsonValue;

public BaseType interfaceWithDeclaredSubtypes;

@JsonUnwrapped
Expand Down Expand Up @@ -131,6 +133,15 @@ enum TestEnumWithJsonPropertyAnnotations {
@JsonProperty Y
}

static class TestTypeWithJsonValue {
String value;

@JsonValue
String getJsonValue() {
return value;
}
Comment on lines +136 to +142
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Let's also consider generic type parameters here.
In that case, it is important what is declared on the specific field/method then, e.g.,

public TestTypeWithJsonValue<String> typeWithJsonValue;
Suggested change
static class TestTypeWithJsonValue {
String value;
@JsonValue
String getJsonValue() {
return value;
}
static class TestTypeWithJsonValue<T> {
T jsonValue;
@JsonValue
T getJsonValue() {
return this.jsonValue;
}

For that to be respected, we'd also need to consider fields and methods explicitly (as only there, the necessary type parameter may be defined) – on top of the one for the type in general.
A generally registered SubtypeResolver may take care of this though (I think, but haven't tested it now).

}

@JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
static class TestTypeWithObjectId {
IdType id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ private void verifyCommonConfigurations(boolean considerNamingStrategy, int addi
Mockito.verify(this.methodConfigPart).withWriteOnlyCheck(Mockito.any());

Mockito.verify(this.typesInGeneralConfigPart).withDescriptionResolver(Mockito.any());
Mockito.verify(this.typesInGeneralConfigPart, Mockito.times(1 + additionalCustomTypeDefinitions))
Mockito.verify(this.typesInGeneralConfigPart, Mockito.times(2 + additionalCustomTypeDefinitions))
.withCustomDefinitionProvider(Mockito.any());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
"type": "string",
"enum": ["entry1", "entry2", "entry3"]
},
"typeWithJsonValue":{
"type":"string"
},
"fieldWithDescription": {
"type": "string",
"description": "field description"
Expand Down
Loading