fix: Track visited classes in catalog generator. (#1117)

Fixes #1116
michael-simons · Oct 31, 2023 · 5b8e1aa · 5b8e1aa
1 parent e1f0644
commit 5b8e1aa
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 15 deletions.
diff --git a/extensions/neo4j-migrations-annotation-processing/processor/pom.xml b/extensions/neo4j-migrations-annotation-processing/processor/pom.xml
@@ -94,6 +94,7 @@
 				<directory>src/test/java</directory>
 				<includes>
 					<include>ac/simons/neo4j/migrations/annotations/proc/catalog/**/*.java</include>
+					<include>ac/simons/neo4j/migrations/annotations/proc/misc/**/*.java</include>
 					<include>ac/simons/neo4j/migrations/annotations/proc/ogm/**/*.java</include>
 					<include>ac/simons/neo4j/migrations/annotations/proc/ogm_invalid/**/*.java</include>
 					<include>ac/simons/neo4j/migrations/annotations/proc/ogm_movies/**/*.java</include>

diff --git a/...ain/java/ac/simons/neo4j/migrations/annotations/proc/impl/CatalogGeneratingProcessor.java b/...ain/java/ac/simons/neo4j/migrations/annotations/proc/impl/CatalogGeneratingProcessor.java
@@ -776,7 +776,7 @@ private void processOGMIdAnnotations(RoundEnvironment roundEnv) {
 					new DefaultNodeType(t.getQualifiedName().toString(), labels));
 				String name = this.constraintNameGenerator.generateName(Constraint.Type.UNIQUE,
 					Collections.singleton(idProperty));
-				catalogItems.add(Constraint.forNode(labels.get(0).getValue()).named(name)
+				catalogItems.add(Constraint.forNode(labels.stream().map(SchemaName::getValue).findFirst().orElseGet(() -> t.getSimpleName().toString())).named(name)
 					.unique(idProperty.getName()));
 			});
 	}
@@ -1133,6 +1133,8 @@ class RequiresPrimaryKeyConstraintPredicate extends ElementKindVisitor8<Boolean,
 
 		private final String internalIdGeneratorClass;
 
+		private final Set<TypeElement> processed = new HashSet<>();
+
 		RequiresPrimaryKeyConstraintPredicate(Collection<TypeElement> idAnnotations, TypeElement generatedValueAnnotation, String generatorAttributeName, String internalIdGeneratorClass) {
 			this.idAnnotations = idAnnotations;
 			this.generatedValueAnnotation = generatedValueAnnotation;
@@ -1147,6 +1149,10 @@ protected Boolean defaultAction(Element e, Boolean aBoolean) {
 
 		@Override
 		public Boolean visitType(TypeElement e, Boolean includeAbstractClasses) {
+			if (!processed.add(e)) {
+				return false;
+			}
+
 			boolean isNonAbstractClass = e.getKind().isClass() && !e.getModifiers().contains(Modifier.ABSTRACT);
 			if (!isNonAbstractClass && Boolean.FALSE.equals(includeAbstractClasses)) {
 				return false;

diff --git a/...java/ac/simons/neo4j/migrations/annotations/proc/impl/CatalogGeneratingProcessorTest.java b/...java/ac/simons/neo4j/migrations/annotations/proc/impl/CatalogGeneratingProcessorTest.java
@@ -196,6 +196,39 @@ void shouldDealWithClassesAndRecords(String classFile) throws IOException {
 			.isEqualTo(expectedCatalog);
 	}
 
+	@Test // GH-1116
+	void classesNestedInTheirNonAbstractParentsMustNotCauseSOException() throws IOException {
+
+		CatalogGeneratingProcessor catalogGeneratingProcessor = new CatalogGeneratingProcessor();
+		Compilation compilation = getCompiler()
+			.withProcessors(catalogGeneratingProcessor)
+			.withOptions(String.format("-Aorg.neo4j.migrations.catalog_generator.timestamp=%s", "2022-12-09T21:21:00+01:00"))
+			.compile(JavaFileObjects.forResource(resourceResolver.getResources(String.format("ac/simons/neo4j/migrations/annotations/proc/misc/%s.java", "Fun"))[0].getURL()));
+
+		var expectedCatalog = """
+			<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+			<migration xmlns="https://michael-simons.github.io/neo4j-migrations">
+			    <!-- This file was generated by Neo4j-Migrations at 2022-12-09T21:21:00+01:00. -->
+			    <catalog>
+			        <indexes/>
+			        <constraints>
+			            <constraint name="ac_simons_neo4j_migrations_annotations_proc_misc_fun_funfun_uuid_unique" type="unique">
+			                <label>FunFun</label>
+			                <properties>
+			                    <property>uuid</property>
+			                </properties>
+			            </constraint>
+			        </constraints>
+			    </catalog>
+			    <apply/>
+			</migration>
+			""";
+		assertThat(compilation)
+			.generatedFile(StandardLocation.SOURCE_OUTPUT, "neo4j-migrations", CatalogGeneratingProcessor.DEFAULT_MIGRATION_NAME)
+			.contentsAsString(StandardCharsets.UTF_8)
+			.isEqualTo(expectedCatalog);
+	}
+
 	static class CollectingConstraintNameGenerator implements ConstraintNameGenerator {
 
 		Map<String, List<String>> labels = new HashMap<>();

diff --git a/...cessing/processor/src/test/java/ac/simons/neo4j/migrations/annotations/proc/misc/Fun.java b/...cessing/processor/src/test/java/ac/simons/neo4j/migrations/annotations/proc/misc/Fun.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2020-2023 the original author or authors.
+ *
+ * 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
+ *
+ *      https://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 ac.simons.neo4j.migrations.annotations.proc.misc;
+
+import org.neo4j.ogm.annotation.Id;
+import org.neo4j.ogm.annotation.NodeEntity;
+
+/**
+ * @author Michael J. Simons
+ */
+@NodeEntity
+public abstract class Fun {
+
+	@Id
+	String uuid;
+
+	static class FunFun extends Fun {
+	}
+}
diff --git a/extensions/neo4j-migrations-formats-adoc/pom.xml b/extensions/neo4j-migrations-formats-adoc/pom.xml
@@ -129,7 +129,7 @@
 						</excludes>
 					</artifactSet>
 					<transformers>
-						<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer" />
+						<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer"/>
 					</transformers>
 					<filters>
 						<filter>

diff --git a/extensions/neo4j-migrations-formats-markdown/pom.xml b/extensions/neo4j-migrations-formats-markdown/pom.xml
@@ -103,7 +103,7 @@
 						</excludes>
 					</artifactSet>
 					<transformers>
-						<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer" />
+						<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer"/>
 					</transformers>
 					<filters>
 						<filter>

diff --git a/neo4j-migrations-cli/pom.xml b/neo4j-migrations-cli/pom.xml
@@ -30,10 +30,10 @@
 	<description>CLI module, packaged as JVM and native assemblies.</description>
 
 	<properties>
-		<assembly-suffix />
+		<assembly-suffix/>
 		<covered-ratio-complexity>0.1</covered-ratio-complexity>
 		<covered-ratio-instructions>0.03</covered-ratio-instructions>
-		<executable-suffix />
+		<executable-suffix/>
 		<java-module-name>ac.simons.neo4j.migrations.cli</java-module-name>
 		<name-of-main-class>ac.simons.neo4j.migrations.cli.MigrationsCli</name-of-main-class>
 		<skipCompress>true</skipCompress>
@@ -118,7 +118,7 @@
 					<reuseForks>false</reuseForks>
 					<environmentVariables>
 						<superSecretSuperPassword>Geheim</superSecretSuperPassword>
-						<emptySecret />
+						<emptySecret/>
 					</environmentVariables>
 					<useModulePath>false</useModulePath>
 				</configuration>
@@ -164,7 +164,7 @@
 							<executable>java</executable>
 							<arguments>
 								<argument>-classpath</argument>
-								<classpath />
+								<classpath/>
 								<argument>${name-of-main-class}</argument>
 								<argument>generate-completion</argument>
 							</arguments>

diff --git a/neo4j-migrations-examples/neo4j-migrations-examples-sb-testharness/pom.xml b/neo4j-migrations-examples/neo4j-migrations-examples-sb-testharness/pom.xml
@@ -163,7 +163,7 @@
 				<configuration>
 					<skip>true</skip>
 				</configuration>
-				<reportSets />
+				<reportSets/>
 			</plugin>
 		</plugins>
 	</reporting>

diff --git a/neo4j-migrations-examples/neo4j-migrations-examples-sb/pom.xml b/neo4j-migrations-examples/neo4j-migrations-examples-sb/pom.xml
@@ -197,7 +197,7 @@
 				<configuration>
 					<skip>true</skip>
 				</configuration>
-				<reportSets />
+				<reportSets/>
 			</plugin>
 		</plugins>
 	</reporting>

diff --git a/neo4j-migrations-maven-plugin/pom.xml b/neo4j-migrations-maven-plugin/pom.xml
@@ -330,7 +330,7 @@
 				<configuration>
 					<skip>true</skip>
 				</configuration>
-				<reportSets />
+				<reportSets/>
 			</plugin>
 			<plugin>
 				<groupId>org.apache.maven.plugins</groupId>

diff --git a/pom.xml b/pom.xml
@@ -117,7 +117,7 @@
 		<guava.version>32.1.3-jre</guava.version>
 		<jacoco-maven-plugin.version>0.8.10</jacoco-maven-plugin.version>
 		<japicmp-maven-plugin.version>0.18.1</japicmp-maven-plugin.version>
-		<java-module-name />
+		<java-module-name/>
 		<java.version>17</java.version>
 		<junit-jupiter-causal-cluster-testcontainer-extension.version>2022.1.8</junit-jupiter-causal-cluster-testcontainer-extension.version>
 		<!-- to be overridden in sub modules -->
@@ -792,7 +792,7 @@
 								<requireJavaVersion>
 									<version>${java.version}</version>
 								</requireJavaVersion>
-								<DependencyConvergence />
+								<DependencyConvergence/>
 								<requireMavenVersion>
 									<version>${maven.version}</version>
 								</requireMavenVersion>
@@ -927,9 +927,9 @@
 					<attributes>
 						<icons>font</icons>
 						<toc>left</toc>
-						<setanchors />
-						<idprefix />
-						<idseparator />
+						<setanchors/>
+						<idprefix/>
+						<idseparator/>
 						<imagesdir>${docsImagesDir}</imagesdir>
 						<neo4j-java-driver-version>${neo4j-java-driver.version}</neo4j-java-driver-version>
 						<neo4j-migrations.version>${project.version}</neo4j-migrations.version>