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.

Sign up for GitHub

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

Jump to bottom

task: improve the cycle error message #5911

Merged
merged 1 commit into from
Apr 17, 2024
Merged

task: improve the cycle error message #5911

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 18 additions & 22 deletions crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -228,7 +228,7 @@ public List<KubernetesValidationRule> getValidationRules() {
*/
protected T internalFrom(TypeDef definition, String... ignore) {
InternalSchemaSwaps schemaSwaps = new InternalSchemaSwaps();
return internalFromImpl(definition, new HashSet<>(), schemaSwaps, ignore);
return internalFromImpl(definition, new LinkedHashMap<>(), schemaSwaps, ignore);
}

private static ClassRef extractClassRef(Object type) {
Expand Down Expand Up @@ -294,7 +294,8 @@ private static Stream<KubernetesValidationRule> extractKubernetesValidationRules
}
}

private T internalFromImpl(TypeDef definition, Set<String> visited, InternalSchemaSwaps schemaSwaps, String... ignore) {
private T internalFromImpl(TypeDef definition, LinkedHashMap<String, String> visited, InternalSchemaSwaps schemaSwaps,
String... ignore) {
Set<String> ignores = ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore))
: Collections
.emptySet();
Expand Down Expand Up @@ -327,9 +328,9 @@ private T internalFromImpl(TypeDef definition, Set<String> visited, InternalSche

schemaSwaps = schemaSwaps.branchDepths();
SwapResult swapResult = schemaSwaps.lookupAndMark(definition.toReference(), name);
LinkedHashMap<String, String> savedVisited = visited;
if (swapResult.onGoing) {
// hack to prevent cycle detection - this may need improved at some point should stackoverflows be encountered
visited.clear();
visited = new LinkedHashMap<>();
}
final PropertyFacade facade = new PropertyFacade(property, accessors, swapResult.classRef);
final Property possiblyRenamedProperty = facade.process();
Expand All @@ -341,6 +342,7 @@ private T internalFromImpl(TypeDef definition, Set<String> visited, InternalSche
continue;
}
final T schema = internalFromImpl(name, possiblyRenamedProperty.getTypeRef(), visited, schemaSwaps);
visited = savedVisited;
if (facade.preserveUnknownFields) {
preserveUnknownFields = true;
}
Expand Down Expand Up @@ -784,10 +786,11 @@ public abstract T build(B builder,
* @return the structural schema associated with the specified property
*/
public T internalFrom(String name, TypeRef typeRef) {
return internalFromImpl(name, typeRef, new HashSet<>(), new InternalSchemaSwaps());
return internalFromImpl(name, typeRef, new LinkedHashMap<>(), new InternalSchemaSwaps());
}

private T internalFromImpl(String name, TypeRef typeRef, Set<String> visited, InternalSchemaSwaps schemaSwaps) {
private T internalFromImpl(String name, TypeRef typeRef, LinkedHashMap<String, String> visited,
InternalSchemaSwaps schemaSwaps) {
// Note that ordering of the checks here is meaningful: we need to check for complex types last
// in case some "complex" types are handled specifically
if (typeRef.getDimensions() > 0 || io.sundr.model.utils.Collections.isCollection(typeRef)) { // Handle Collections & Arrays
Expand Down Expand Up @@ -848,24 +851,17 @@ private T internalFromImpl(String name, TypeRef typeRef, Set<String> visited, In
}
}

// Flag to detect cycles
private boolean resolving = false;

private T resolveNestedClass(String name, TypeDef def, Set<String> visited, InternalSchemaSwaps schemaSwaps) {
if (!resolving) {
visited.clear();
resolving = true;
} else {
String visitedName = name + ":" + def.getFullyQualifiedName();
if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(visitedName)) {
throw new IllegalArgumentException(
"Found a cyclic reference involving the field " + name + " of type " + def.getFullyQualifiedName());
}
visited.add(visitedName);
private T resolveNestedClass(String name, TypeDef def, LinkedHashMap<String, String> visited,
InternalSchemaSwaps schemaSwaps) {
if (visited.put(def.getFullyQualifiedName(), name) != null) {
throw new IllegalArgumentException(
"Found a cyclic reference involving the field of type " + def.getFullyQualifiedName() + " starting a field "
+ visited.entrySet().stream().map(e -> e.getValue() + " >>\n" + e.getKey()).collect(Collectors.joining(".")) + "."
+ name);
}

T res = internalFromImpl(def, visited, schemaSwaps);
resolving = false;
visited.remove(def.getFullyQualifiedName());
return res;
}

Expand Down