diff --git a/etc/adr/general-discussion.adoc b/etc/adr/general-discussion.adoc index 7bab300e0..b156a6ceb 100644 --- a/etc/adr/general-discussion.adoc +++ b/etc/adr/general-discussion.adoc @@ -238,16 +238,8 @@ core-[hidden]--->repository |=== === Architecture validation -The structure of this project can be explored as a Graph. -We use https://jqassistant.org[jQAssistant] to verify our architecture during the build. -Run the following two commands - -``` -./mvnw clean compile jqassistant:scan -./mvnw jqassistant:server -``` - -and point your browser to http://localhost:7474. +In favour of lightweight builds and JDK restriction of Neo4j, we moved away from https://jqassistant.org[jQAssistant] (still a great tool) and +have now https://www.archunit.org[ArchUnit] in place. === `SimpleNeo4jRepository` initialization . `@EnableNeo4jRepositories` defines diff --git a/etc/jqassistant/api.adoc b/etc/jqassistant/api.adoc deleted file mode 100644 index bdb164fb8..000000000 --- a/etc/jqassistant/api.adoc +++ /dev/null @@ -1,53 +0,0 @@ -[[api:Default]] -[role=group,includesConstraints="api:*"] - -=== General considerations - -We use https://github.com/apiguardian-team/apiguardian[@API Guardian] to keep track of what we expose as public or internal API. -To keep things both clear and concise, we restrict the usage of those annotations to interfaces, classes (incl. constructors) -and annotations. - -[[api:api-guardian-usage]] -[source,cypher,role="constraint"] -.@API Guardian annotations must not be used on fields ----- -MATCH (c:Java)-[:ANNOTATED_BY]->(a)-[:OF_TYPE]->(t:Type {fqn: 'org.apiguardian.api.API'}), - (p)-[:DECLARES]->(c) -WHERE c:Member AND NOT (c:Constructor OR c:Method) -RETURN p.fqn, c.name ----- - -Public interfaces, classes or annotations are either part of internal or public API and have a status. - -[[api:api-guardian-api-concept]] -[source,cypher,role="concept",verify=rowCount,rowCountMin=0] -.Define which Java artifacts are part of internal or public API ----- -MATCH (c:Java)-[:ANNOTATED_BY]->(a)-[:OF_TYPE]->(t:Type {fqn: 'org.apiguardian.api.API'}), - (a)-[:HAS]->({name: 'status'})-[:IS]->(s) -WHERE ANY (label IN labels(c) WHERE label in ['Interface', 'Class', 'Annotation']) -WITH c, trim(split(s.signature, ' ')[1]) AS status -WITH c, status, - CASE status - WHEN 'INTERNAL' THEN 'Internal' - ELSE 'Public' - END AS type -MERGE (a:Api {type: type, status: status}) -MERGE (c)-[:IS_PART_OF]->(a) -RETURN c,a ----- - -=== Internal API - -See ADR-003. - -[[api:internal]] -[source,cypher,role="constraint",requiresConcepts="api:api-guardian-api-concept"] -.Non abstract, public classes that are only part of internal API must be final ----- -MATCH (c:Class)-[:IS_PART_OF]->(:Api {type: 'Internal'}) -WHERE c.visibility = 'public' - AND coalesce(c.abstract, false) = false - AND NOT exists(c.final) -RETURN c.name ----- diff --git a/etc/jqassistant/index.adoc b/etc/jqassistant/index.adoc deleted file mode 100644 index a42b41481..000000000 --- a/etc/jqassistant/index.adoc +++ /dev/null @@ -1,27 +0,0 @@ -[[coding-rules]] -= Coding Rules - -The following rules are checked during a build: - -[[default]] -[role=group,includesGroups="api:Default,naming:Default,structure:Default"] -- <> -- <> -- <> - -[[coding-rules.api]] -== API - -Ensure that we publish our API in a sane and consistent way. - -include::api.adoc[] - -[[coding-rules.naming]] -== Naming things - -include::naming.adoc[] - -[[coding-rules.structure]] -== Structuring things - -include::structure.adoc[] diff --git a/etc/jqassistant/naming.adoc b/etc/jqassistant/naming.adoc deleted file mode 100644 index b8b760c84..000000000 --- a/etc/jqassistant/naming.adoc +++ /dev/null @@ -1,16 +0,0 @@ -[[naming:Default]] -[role=group,includesConstraints="naming:TypeNameMustBeginWithGroupId"] - -The following naming conventions are used throughout the project: - -[[naming:TypeNameMustBeginWithGroupId]] -[source,cypher,role=constraint] -.All Java types must be located in packages that start with `org.springframework.data.neo4j`. ----- -MATCH - (project:Maven:Project)-[:CREATES]->(:Artifact)-[:CONTAINS]->(type:Type) -WHERE - NOT type.fqn starts with 'org.springframework.data.neo4j' -RETURN - project as Project, collect(type) as TypeWithWrongName ----- diff --git a/etc/jqassistant/structure.adoc b/etc/jqassistant/structure.adoc deleted file mode 100644 index 36e2052bb..000000000 --- a/etc/jqassistant/structure.adoc +++ /dev/null @@ -1,37 +0,0 @@ -[[structure:Default]] -[role=group,includesConstraints="structure:mapping,structure:support-packages"] - -Most of the time, the package structure under `org.springframework.data.neo4j` should reflect the main building parts. - -[[structure:mapping]] -[source,cypher,role=constraint,requiresConcepts="dependency:Package"] -.The mapping package must not depend on any other SDN packages than `schema` and `convert` ----- -MATCH (a:Main:Artifact) -OPTIONAL MATCH (a)-[:CONTAINS]->(s:Package) WHERE s.fqn in ['org.springframework.data.neo4j.core.schema', 'org.springframework.data.neo4j.core.convert'] -WITH collect(s) as allowed, a -MATCH (a)-[:CONTAINS]->(p1:Package)-[:DEPENDS_ON]->(p2:Package)<-[:CONTAINS]-(a) -WHERE p1.fqn = 'org.springframework.data.neo4j.core.mapping' - AND NOT (p2 in allowed OR (p1) -[:CONTAINS]-> (p2)) -RETURN p1,p2 ----- - -[[structure:support-packages]] -[source,cypher,role=constraint,requiresConcepts="dependency:Package"] -.The public support packages must not depend directly on the mapping package ----- -MATCH (a:Main:Artifact) -MATCH (a)-[:CONTAINS]->(p1:Package) -WHERE p1.fqn in [ - 'org.springframework.data.neo4j.core.convert', - 'org.springframework.data.neo4j.core.schema', - 'org.springframework.data.neo4j.core.support', - 'org.springframework.data.neo4j.core.transaction' - ] -WITH p1, a -MATCH (p1)-[:CONTAINS]->(t:Type) -MATCH (t)-[:DEPENDS_ON]->(t2:Type)<-[:CONTAINS]-(p2:Package)<-[:CONTAINS]-(a) -WHERE t2.fqn <> 'org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty' - AND p2.fqn = 'org.springframework.data.neo4j.core.mapping' -RETURN t ----- \ No newline at end of file diff --git a/pom.xml b/pom.xml index e2bb6cb94..f597dc86c 100644 --- a/pom.xml +++ b/pom.xml @@ -69,9 +69,9 @@ 1.1.1 + 0.23.1 2.1.0 2.1.0 - 1.11.0 3.0.1 ${skipTests} @@ -86,10 +86,6 @@ spring.data.neo4j 17 2.3.1 - 1.10.0 - 1.8.0 - 1.10.1 - 1.10.1 2021.0.1 3.1.2 3.0.0-M1 @@ -230,6 +226,11 @@ pom import + + com.tngtech.archunit + archunit + ${archunit.version} + @@ -437,6 +438,11 @@ + + com.tngtech.archunit + archunit + test + @@ -482,27 +488,6 @@ false - - com.buschmais.jqassistant - jqassistant-maven-plugin - ${jqassistant.version} - - - de.kontext-e.jqassistant.plugin - jqassistant.plugin.git - ${jqassistant.plugin.git.version} - - - org.jqassistant.contrib.plugin - jqassistant-dashboard-plugin - ${jqassistant-dashboard-plugin.version} - - - - etc/jqassistant - ${skipArchitectureTests} - - org.jacoco jacoco-maven-plugin @@ -712,45 +697,6 @@ - - com.buschmais.jqassistant - jqassistant-maven-plugin - - - jqassistant-scan - verify - - scan - - - - jacoco.xml - - - - ${project.basedir}/.git - - - ${project.reporting.outputDirectory}/jacoco - - - - - - jqassistant-analyze - - analyze - - - MINOR - - default - jqassistant-dashboard:Default - - - - - org.apache.maven.plugins maven-compiler-plugin diff --git a/src/main/asciidoc/appendix/build.adoc b/src/main/asciidoc/appendix/build.adoc index 0ac1b693f..be967f377 100644 --- a/src/main/asciidoc/appendix/build.adoc +++ b/src/main/asciidoc/appendix/build.adoc @@ -152,46 +152,3 @@ There is no quality gate in place at the moment to ensure that the code/test rat We have some rather mild checkstyle rules in place, enforcing more or less default Java formatting rules. Your build will break on formatting errors or something like unused imports. -[[building-SDN.jqassistant]] -=== jQAssistant - -WARNING: Verification of those rules is off by default since Spring Data Neo4j 7, as the it requires JDK 17 to build. - The currently available version of jQAssistant does not run without several `--add-opens` arguments to the Java - module system and we won't apply this to the whole build as we must ensure that SDN itself does *not* need them. - + - To execute the rules during a build run the build like this: - + - + - `MAVEN_OPTS="--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED" ./mvnw -DskipArchitectureTests=false -DskipTests=true verify` - - -We also use https://jqassistant.org[jQAssistant], a Neo4j-based tool, to verify some aspects of our architecture. -The rules are described with Cypher and your build will break when they are violated: - -include::../../../../etc/jqassistant/index.adoc[leveloffset=4] - -[[building-SDN.jqassistant.database]] -==== Accessing the jQAssistant database - -jQAssistant uses Neo4j to store information about a project. -To access the database, please build the project as described above. -When the build finishes, execute the following command: - -[source,console,subs="verbatim,attributes"] -[[start-jqassistant]] -.Start jQAssistant ----- -$ MAVEN_OPTS="--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED" ./mvnw -DskipArchitectureTests=false jqassistant:server ----- - -Access the standard Neo4j browser at http://localhost:7474 and a dedicated jQA-Dashboard at http://localhost:7474/jqassistant/dashboard/. - -The scanning and analyzing can be triggered individually, without going through the full verify again: - -[source,console,subs="verbatim,attributes"] -[[scan-with-jqassistant]] -.Manually scan and analyze the main project ----- -$ MAVEN_OPTS="--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED" ./mvnw -DskipArchitectureTests=false jqassistant:scan@jqassistant-scan -$ MAVEN_OPTS="--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED" ./mvnw -DskipArchitectureTests=false jqassistant:analyze@jqassistant-analyze ----- diff --git a/src/test/java/org/springframework/data/neo4j/architecture/ArchitectureTest.java b/src/test/java/org/springframework/data/neo4j/architecture/ArchitectureTest.java new file mode 100644 index 000000000..a248bf969 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/architecture/ArchitectureTest.java @@ -0,0 +1,116 @@ +/* + * Copyright 2011-2022 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 org.springframework.data.neo4j.architecture; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.domain.JavaModifier; +import com.tngtech.archunit.core.domain.properties.HasModifiers; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +import com.tngtech.archunit.library.Architectures; +import org.apiguardian.api.API; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +/** + * Architecture tests replacing the jQAssistant tests. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class ArchitectureTest { + + private static final DescribedPredicate INTERNAL_API_PREDICATE = new DescribedPredicate<>("Is internal API") { + @Override + public boolean apply(JavaClass input) { + API.Status status = input.getAnnotationOfType(API.class).status(); + return "INTERNAL".equals(status.name()); + } + }; + + private JavaClasses sdnClasses; + + @BeforeAll + void importCorePackage() { + sdnClasses = new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages("org.springframework.data.neo4j.."); + + + } + + @DisplayName("Non abstract, public classes that are only part of internal API must be final") + @Test + void finalInternalAPIPublicClasses() { + ArchRule rule = ArchRuleDefinition.classes().that().areAnnotatedWith(API.class) + .and().arePublic() + .and().areTopLevelClasses() + .and(DescribedPredicate.not(HasModifiers.Predicates.modifier(JavaModifier.ABSTRACT))) + .and(INTERNAL_API_PREDICATE) + .should().haveModifier(JavaModifier.FINAL); + rule.check(sdnClasses); + } + + + @DisplayName("@API Guardian annotations must not be used on fields") + @Test + void apiAnnotationsNotOnFields() { + + ArchRule rule = ArchRuleDefinition.fields().should() + .notBeAnnotatedWith(API.class); + + rule.check(sdnClasses); + } + + @DisplayName("The mapping package must not depend on any other SDN packages than schema and convert") + @Test + void mappingPackageDependencies() { + + Architectures.layeredArchitecture() + .layer("mapping").definedBy("org.springframework.data.neo4j.core.mapping", "org.springframework.data.neo4j.core.mapping.callback") + .layer("schema or conversion").definedBy("org.springframework.data.neo4j.core.schema", "org.springframework.data.neo4j.core.convert") + .layer("everything outside SDN").definedBy(new DescribedPredicate<>("classes outside SDN") { + @Override + public boolean apply(JavaClass input) { + return !input.getPackageName().startsWith("org.springframework.data.neo4j"); + } + }) + .whereLayer("mapping").mayOnlyAccessLayers("schema or conversion", "everything outside SDN") + .withOptionalLayers(true) + .check(sdnClasses); + } + + @DisplayName("The public support packages must not depend directly on the mapping package") + @Test + void publicPackagesMustNotDependOnMappingPackage() { + + ArchRuleDefinition.classes().that().resideInAnyPackage("org.springframework.data.neo4j.core.convert", + "org.springframework.data.neo4j.core.schema", + "org.springframework.data.neo4j.core.support", + "org.springframework.data.neo4j.core.transaction") + .should() + .onlyDependOnClassesThat() + .resideOutsideOfPackages("org.springframework.data.neo4j.core.mapping", "org.springframework.data.neo4j.core.mapping.callback") + .orShould() + .dependOnClassesThat().haveFullyQualifiedName("org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty") + .check(sdnClasses); + + } +}