Skip to content

Commit

Permalink
chore: further refactoring for code health improvement (#406)
Browse files Browse the repository at this point in the history
* chore: further refactor maven plugin

* chore: refactoring of generation context

* feat: add cache for types with members

* chore: reduce GlobHandler complexity
  • Loading branch information
CarstenWickner authored Nov 15, 2023
1 parent c1744cf commit 0daa090
Show file tree
Hide file tree
Showing 13 changed files with 560 additions and 344 deletions.
2 changes: 1 addition & 1 deletion checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@
<!--<module name="FinalParameters" />-->
<module name="InnerAssignment" />
<module name="CyclomaticComplexity">
<property name="max" value="16"/>
<property name="max" value="9"/>
<property name="switchBlockAsSingleDecisionPoint" value="true"/>
</module>
</module>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,25 +133,9 @@ private FieldScope doFindGetterField() {
String methodName = this.getDeclaredName();
Set<String> possibleFieldNames = new HashSet<>(3);
if (methodName.startsWith("get")) {
if (methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) {
// ensure that the variable starts with a lower-case letter
possibleFieldNames.add(methodName.substring(3, 4).toLowerCase() + methodName.substring(4));
}
// @since 4.32.0 - conforming with JavaBeans API specification edge case when second character in field name is in uppercase
if (methodName.length() > 4 && Character.isUpperCase(methodName.charAt(4))) {
possibleFieldNames.add(methodName.substring(3));
}
getPossibleFieldNamesStartingWithGet(methodName, possibleFieldNames);
} else if (methodName.startsWith("is")) {
if (methodName.length() > 2 && Character.isUpperCase(methodName.charAt(2))) {
// ensure that the variable starts with a lower-case letter
possibleFieldNames.add(methodName.substring(2, 3).toLowerCase() + methodName.substring(3));
// since 4.32.0: a method "isBool()" is considered a possible getter for a field "isBool" as well as for "bool"
possibleFieldNames.add(methodName);
}
// @since 4.32.0 - conforming with JavaBeans API specification edge case when second character in field name is in uppercase
if (methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) {
possibleFieldNames.add(methodName.substring(2));
}
getPossibleFieldNamesStartingWithIs(methodName, possibleFieldNames);
}
if (possibleFieldNames.isEmpty()) {
// method name does not fall into getter conventions
Expand All @@ -166,6 +150,30 @@ private FieldScope doFindGetterField() {
.orElse(null);
}

private static void getPossibleFieldNamesStartingWithGet(String methodName, Set<String> possibleFieldNames) {
if (methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) {
// ensure that the variable starts with a lower-case letter
possibleFieldNames.add(methodName.substring(3, 4).toLowerCase() + methodName.substring(4));
}
// @since 4.32.0 - conforming with JavaBeans API specification edge case when second character in field name is in uppercase
if (methodName.length() > 4 && Character.isUpperCase(methodName.charAt(4))) {
possibleFieldNames.add(methodName.substring(3));
}
}

private static void getPossibleFieldNamesStartingWithIs(String methodName, Set<String> possibleFieldNames) {
if (methodName.length() > 2 && Character.isUpperCase(methodName.charAt(2))) {
// ensure that the variable starts with a lower-case letter
possibleFieldNames.add(methodName.substring(2, 3).toLowerCase() + methodName.substring(3));
// since 4.32.0: a method "isBool()" is considered a possible getter for a field "isBool" as well as for "bool"
possibleFieldNames.add(methodName);
}
// @since 4.32.0 - conforming with JavaBeans API specification edge case when second character in field name is in uppercase
if (methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) {
possibleFieldNames.add(methodName.substring(2));
}
}

/**
* Determine whether the method's name matches the getter naming convention ("getFoo()"/"isFoo()") and a respective field ("foo") exists.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -212,7 +213,6 @@ private ObjectNode buildDefinitionsAndResolveReferences(String designatedDefinit
createDefinitionsForAll, inlineAllSchemas);
Map<DefinitionKey, String> baseReferenceKeys = this.getReferenceKeys(mainSchemaKey, shouldProduceDefinition, generationContext);
considerOnlyDirectReferences.set(true);
final boolean createDefinitionForMainSchema = this.config.shouldCreateDefinitionForMainSchema();
for (Map.Entry<DefinitionKey, String> entry : baseReferenceKeys.entrySet()) {
String definitionName = entry.getValue();
DefinitionKey definitionKey = entry.getKey();
Expand All @@ -227,13 +227,11 @@ private ObjectNode buildDefinitionsAndResolveReferences(String designatedDefinit
referenceKey = null;
} else {
// the same sub-schema is referenced in multiple places
if (createDefinitionForMainSchema || !definitionKey.equals(mainSchemaKey)) {
// add it to the definitions (unless it is the main schema that is not explicitly moved there via an Option)
definitionsNode.set(definitionName, generationContext.getDefinition(definitionKey));
referenceKey = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/' + definitionName;
} else {
referenceKey = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN);
}
Supplier<String> addDefinitionAndReturnReferenceKey = () -> {
definitionsNode.set(definitionName, this.generationContext.getDefinition(definitionKey));
return this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/' + definitionName;
};
referenceKey = getReferenceKey(mainSchemaKey, definitionKey, addDefinitionAndReturnReferenceKey);
references.forEach(node -> node.put(this.config.getKeyword(SchemaKeyword.TAG_REF), referenceKey));
}
if (!nullableReferences.isEmpty()) {
Expand All @@ -260,6 +258,18 @@ private ObjectNode buildDefinitionsAndResolveReferences(String designatedDefinit
return definitionsNode;
}

private String getReferenceKey(DefinitionKey mainSchemaKey, DefinitionKey definitionKey, Supplier<String> addDefinitionAndReturnReferenceKey) {
final String referenceKey;
if (definitionKey.equals(mainSchemaKey) && !this.config.shouldCreateDefinitionForMainSchema()) {
// no need to add the main schema into the definitions, unless explicitly configured to do so
referenceKey = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN);
} else {
// add it to the definitions
referenceKey = addDefinitionAndReturnReferenceKey.get();
}
return referenceKey;
}

/**
* Produce reusable predicate for checking whether a given type should produce an entry in the {@link SchemaKeyword#TAG_DEFINITIONS} or not.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.github.victools.jsonschema.generator.impl.Util;
import com.github.victools.jsonschema.generator.naming.SchemaDefinitionNamingStrategy;
import java.lang.annotation.Annotation;
import java.math.BigDecimal;
Expand Down Expand Up @@ -361,7 +362,7 @@ CustomDefinition getCustomDefinition(ResolvedType javaType, SchemaGenerationCont
@Deprecated
default ResolvedType resolveTargetTypeOverride(FieldScope field) {
List<ResolvedType> result = this.resolveTargetTypeOverrides(field);
return result == null || result.isEmpty() ? null : result.get(0);
return Util.isNullOrEmpty(result) ? null : result.get(0);
}

/**
Expand All @@ -374,7 +375,7 @@ default ResolvedType resolveTargetTypeOverride(FieldScope field) {
@Deprecated
default ResolvedType resolveTargetTypeOverride(MethodScope method) {
List<ResolvedType> result = this.resolveTargetTypeOverrides(method);
return result == null || result.isEmpty() ? null : result.get(0);
return Util.isNullOrEmpty(result) ? null : result.get(0);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.WeakHashMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -44,6 +45,7 @@ public class TypeContext {

private final TypeResolver typeResolver;
private final MemberResolver memberResolver;
private final WeakHashMap<ResolvedType, ResolvedTypeWithMembers> typesWithMembersCache;
private final AnnotationConfiguration annotationConfig;
private final boolean derivingFieldsFromArgumentFreeMethods;

Expand Down Expand Up @@ -87,6 +89,7 @@ private TypeContext(AnnotationConfiguration annotationConfig, boolean derivingFi
this.memberResolver = new MemberResolver(this.typeResolver);
this.annotationConfig = annotationConfig;
this.derivingFieldsFromArgumentFreeMethods = derivingFieldsFromArgumentFreeMethods;
this.typesWithMembersCache = new WeakHashMap<>();
}

/**
Expand Down Expand Up @@ -129,6 +132,16 @@ public final ResolvedType resolveSubtype(ResolvedType supertype, Class<?> subtyp
* @return collection of (resolved) fields and methods
*/
public final ResolvedTypeWithMembers resolveWithMembers(ResolvedType resolvedType) {
return this.typesWithMembersCache.computeIfAbsent(resolvedType, this::resolveWithMembersForCache);
}

/**
* Collect a given type's declared fields and methods for the inclusion in the internal cache.
*
* @param resolvedType type for which to collect declared fields and methods
* @return collection of (resolved) fields and methods
*/
private ResolvedTypeWithMembers resolveWithMembersForCache(ResolvedType resolvedType) {
return this.memberResolver.resolve(resolvedType, this.annotationConfig, null);
}

Expand Down
Loading

0 comments on commit 0daa090

Please sign in to comment.