Merge pull request #8 from ls1intum/feature/github-workflow-maven

Add workflow to build package and run tests

.github/workflows/maven.yml

@@ -0,0 +1,44 @@
+# This workflow will build a Java project with Maven, and cache/restore any dependencies to improve the workflow execution time
+# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-java-with-maven
+
+name: Java CI with Maven
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+
+jobs:
+  build-and-test:
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout Repository
+        uses: actions/checkout@v2
+
+      - name: Set up JDK 21
+        uses: actions/setup-java@v3
+        with:
+          java-version: '21'
+          distribution: 'adopt'
+
+      - name: Install Maven
+        run: |
+          sudo rm -rf /usr/bin/mvn
+          sudo wget -q -O - "https://apache.osuosl.org/maven/maven-3/3.9.8/binaries/apache-maven-3.9.8-bin.tar.gz" | sudo tar xzf - -C /usr/share
+          sudo ln -s /usr/share/apache-maven-3.9.8/bin/mvn /usr/bin/mvn
+
+      - name: Build and Test with Maven
+        run: mvn -B package --file pom.xml
+
+      - name: Run Tests on Student Submission
+        run: |
+          git config --global url.https://github.com/.insteadOf git://github.com/
+          git clone https://github.com/sarpsahinalp/test-student-submission.git
+          mkdir test-student-submission/libs
+          cp target/*.jar test-student-submission/libs
+          cd test-student-submission
+          ./gradlew build
+          ./gradlew test

src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/postcompile/CustomClassResolver.java

@@ -1,37 +1,41 @@
 package de.tum.cit.ase.ares.api.architecturetest.java.postcompile;
 
-import com.tngtech.archunit.ArchConfiguration;
 import com.tngtech.archunit.core.domain.JavaClass;
+import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.importer.ClassFileImporter;
-import com.tngtech.archunit.core.importer.resolvers.ClassResolverFromClasspath;
 
-import java.net.URL;
 import java.util.Optional;
 
 /**
  * Custom class resolver to resolve classes that are outside classpath to be able to analyze them transitively.
  */
 public class CustomClassResolver {
-
-    private CustomClassResolver() {
-        throw new IllegalStateException("Utility class");
-    }
-
     /**
      * Class file importer to import the class files.
      * This is used to import the class files from the URL.
      */
-    private static final ClassFileImporter classFileImporter = new ClassFileImporter();
+    private final JavaClasses allClasses;
+
+    public CustomClassResolver() {
+        // We need to import all classes to be able to resolve them later.
+        // TODO: We definitely need to improve this. We should not import all classes as it is not memory efficient.
+        // https://www.javadoc.io/doc/com.tngtech.archunit/archunit/0.10.2/com/tngtech/archunit/core/importer/ClassFileImporter.html
+        allClasses = new ClassFileImporter()
+                .importClasspath();
+    }
 
     /**
      * Try to resolve the class by the given type name.
      *
      * @param typeName The type name of the class to resolve.
      * @return The resolved class if it exists.
      */
-    public static Optional<JavaClass> tryResolve(String typeName) {
-        ArchConfiguration.get().setClassResolver(ClassResolverFromClasspath.class);
-        URL url = CustomClassResolver.class.getResource("/" + typeName.replace(".", "/") + ".class");
-        return url != null ? Optional.of(classFileImporter.importUrl(url).get(typeName)) : Optional.empty();
+    public Optional<JavaClass> tryResolve(String typeName) {
+        try {
+            // Try to resolve the class by the given type name.
+            return Optional.ofNullable(allClasses.get(typeName));
+        } catch (IllegalArgumentException e) {
+            return Optional.empty();
+        }
     }
 }

src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/postcompile/JavaArchitectureTestCaseCollection.java

@@ -42,14 +42,12 @@ private JavaArchitectureTestCaseCollection() {
             "java.nio.file",
             "java.util.prefs",
             "sun.print",
-            "sun.security",
             "java.util.jar",
             "java.util.zip",
             "sun.awt.X11",
             "javax.imageio",
             "javax.sound.midi",
-            "javax.swing.filechooser",
-            "java.awt.desktop");
+            "javax.swing.filechooser");
 
     /**
      * Load pre file contents

src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/postcompile/TransitivelyAccessesMethodsCondition.java

@@ -12,10 +12,7 @@
 import de.tum.cit.ase.ares.api.architecturetest.java.JavaSupportedArchitectureTestCase;
 
 import java.io.IOException;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
+import java.util.*;
 
 import static com.tngtech.archunit.lang.ConditionEvent.createMessage;
 import static com.tngtech.archunit.thirdparty.com.google.common.base.Preconditions.checkNotNull;
@@ -30,15 +27,37 @@
  */
 public class TransitivelyAccessesMethodsCondition extends ArchCondition<JavaClass> {
 
+    /**
+     * Set of classes that does not lead to a violation if they are accessed
+     */
+    private static final Set<String> bannedClasses = Set.of(
+            "java.lang.Object",
+            "java.lang.String",
+            "java.security.AccessControl",
+            "java.util",
+            "sun.util"
+    );
+
+    /**
+     * Predicate to match the accessed methods
+     */
     private final DescribedPredicate<? super JavaAccess<?>> conditionPredicate;
+
+    /**
+     * Transitive access path to find the path to the accessed method
+     */
     private final TransitiveAccessPath transitiveAccessPath = new TransitiveAccessPath();
 
+    /**
+     * Custom class resolver to resolve classes that are outside classpath to be able to analyze them transitively
+     */
+    private final CustomClassResolver customClassResolver;
+
     /**
      * @param conditionPredicate Predicate to match the accessed methods
      */
     public TransitivelyAccessesMethodsCondition(DescribedPredicate<? super JavaAccess<?>> conditionPredicate, JavaSupportedArchitectureTestCase javaSupportedArchitectureTestCase) {
         super("transitively depend on classes that " + conditionPredicate.getDescription());
-
         switch (javaSupportedArchitectureTestCase) {
             case FILESYSTEM_INTERACTION -> {
                 try {
@@ -55,6 +74,7 @@ public TransitivelyAccessesMethodsCondition(DescribedPredicate<? super JavaAcces
             case null, default -> throw new IllegalStateException("JavaSupportedArchitecture cannot be null");
         }
         this.conditionPredicate = checkNotNull(conditionPredicate);
+        this.customClassResolver = new CustomClassResolver();
     }
 
     /**
@@ -143,18 +163,47 @@ private boolean addAccessesToPathFrom(
          * @return all accesses to the same target as the supplied item that are not in the analyzed classes
          */
         private Set<JavaAccess<?>> getDirectAccessTargetsOutsideOfAnalyzedClasses(JavaAccess<?> item) {
-            Optional<JavaClass> resolvedTarget = CustomClassResolver.tryResolve(item.getTargetOwner().getFullName());
-            return resolvedTarget.map(javaClass -> javaClass.getAccessesFromSelf()
+            // If the target owner is in the banned classes, return an empty set
+            if (bannedClasses.stream().anyMatch(p -> item.getTargetOwner().getFullName().startsWith(p)) || item.getTargetOwner().isAssignableTo(Exception.class) || item.getTargetOwner().isAssignableTo(Error.class)) {
+                return Collections.emptySet();
+            }
+
+            // Get all subclasses of the target owner including the target owner
+            JavaClass resolvedTarget = resolveTargetOwner(item.getTargetOwner());
+
+            // Match the accesses to the target
+            Set<JavaClass> subclasses = resolvedTarget.getSubclasses().stream().map(this::resolveTargetOwner).collect(toSet());
+            subclasses.add(resolvedTarget);
+
+            /**
+             * If the number of subclasses is more than 20, return an empty set.
+             * These classes are always generic interfaces or abstract classes
+             * TODO: Check if this is also the case for foreign packages
+             */
+            if (subclasses.size() > 20) {
+                return Collections.emptySet();
+            }
+
+            return subclasses.stream()
+                    .map(javaClass -> getAccessesFromClass(javaClass, item.getTarget().getName()))
+                    .flatMap(Set::stream)
+                    .collect(toSet());
+        }
+
+        private Set<JavaAccess<?>> getAccessesFromClass(JavaClass javaClass, String methodName) {
+            return javaClass.getAccessesFromSelf()
                     .stream()
                     .filter(a -> a
                             .getOrigin()
-                            .getFullName()
-                            .equals(item
-                                    .getTarget()
-                                    .getFullName()
-                            )
-                    )
-                    .collect(toSet())).orElseGet(Set::of);
+                            .getName()
+                            .equals(methodName))
+                    .filter(a -> bannedClasses.stream().noneMatch(p -> a.getTargetOwner().getFullName().startsWith(p)))
+                    .collect(toSet());
+        }
+
+        private JavaClass resolveTargetOwner(JavaClass targetOwner) {
+            Optional<JavaClass> resolvedTarget = customClassResolver.tryResolve(targetOwner.getFullName());
+            return resolvedTarget.orElse(targetOwner);
         }
     }
 }

src/main/java/de/tum/cit/ase/ares/api/ast/model/RecursionCheck.java

@@ -108,7 +108,7 @@ public static String formatVertexInfoWithinCycle(String vertexName, MethodCallGr
         }
 
         return localized("ast.method.get_formatted_file_string_prefix", pathOfSourceRoot.toString()
-                + "/" + Objects.requireNonNull(extractClassName(vertexName)))
+                + File.separator + Objects.requireNonNull(extractClassName(vertexName)))
                 + System.lineSeparator()
                 + localized("ast.method.get_formatted_unwanted_node_string_prefix",
                 "Unwanted Recursion")

src/main/resources/archunit.properties

@@ -0,0 +1,3 @@
+# Set to false to ignore missing dependencies in the classpath, as they are resolved manually by the de.tum.cit.ase.ares.api.architecturetest.java.postcompile.CustomClassResolver
+resolveMissingDependenciesFromClassPath=false
+archRule.failOnEmptyShould=false

src/main/resources/logback.xml

@@ -12,6 +12,7 @@
 		</encoder>
 	</appender>
 
+	<logger name="com.tngtech.archunit" level="INFO" />
 	<root level="debug">
 		<appender-ref ref="STDOUT" />
 	</root>

src/test/java/de/tum/cit/ase/ares/integration/AstAssertionTest.java

@@ -690,8 +690,7 @@ void test_testExcludesPassedMethods_Success() {
 		void test_testExcludesPassedMethod_Fail() {
 			String testExcludesPassedMethod_Fail = "testExcludesPassedMethod_Fail";
 			tests.assertThatEvents().haveExactly(1, testFailedWith(testExcludesPassedMethod_Fail, AssertionError.class, "Unwanted statement found:" + System.lineSeparator() + " - In "
-					+ Path.of("src", "test", "java", "de", "tum", "cit", "ase", "ares", "integration", "testuser", "subject", "structural", "astTestFiles", "recursions" , "excludeMethods",
-					"ClassWithNoExcludeMethods.java")
+					+ Path.of("src", "test", "java", "de", "tum", "cit", "ase", "ares", "integration", "testuser", "subject", "structural", "astTestFiles", "recursions" , "excludeMethods", "ClassWithNoExcludeMethods.java")
 					+ ":" + System.lineSeparator() + "  - Unwanted Recursion was found:"
 					+ System.lineSeparator() + "   - Between line 5 (column 5) and line 7 (column 5) in Method de.tum.cit.ase.ares.integration.testuser.subject.structural.astTestFiles.recursions.excludeMethods.ClassWithNoExcludeMethods.something(de.tum.cit.ase.ares.integration.testuser.subject.structural.astTestFiles.recursions.excludeMethods.RandomParameterThatShouldBeResolved)" + System.lineSeparator()));
 		}

src/test/java/de/tum/cit/ase/ares/integration/PathAccessTest.java

@@ -99,4 +99,9 @@ void test_accessPathTest() {
 	void test_weAccessPath() {
 		tests.assertThatEvents().haveExactly(1, finishedSuccessfully(weAccessPath));
 	}
+
+	@TestTest
+	void test_accessFileSystem() {
+		tests.assertThatEvents().haveExactly(1, testFailedWith("accessFileSystem", SecurityException.class));
+	}
 }

src/test/java/de/tum/cit/ase/ares/integration/testuser/PathAccessUser.java

@@ -102,8 +102,11 @@ void accessPathTest() throws IOException {
 	}
 
 	@PublicTest
-	@Policy(value = "src/test/resources/de/tum/cit/ase/ares/integration/testuser/securitypolicies/NoAllowedPathPolicy.yaml", withinPath = "test-classes/de/tum/cit/ase/ares/integration/testuser/subject/pathaccess")
 	void weAccessPath() throws IOException {
 		Files.readString(Path.of("pom.xml"));
 	}
+
+	@PublicTest
+	@Policy(value = "src/test/resources/de/tum/cit/ase/ares/integration/testuser/securitypolicies/NoAllowedPathPolicy.yaml", withinPath = "test-classes/de/tum/cit/ase/ares/integration/testuser/subject/student")
+	void accessFileSystem() throws IOException {}
 }

src/test/java/de/tum/cit/ase/ares/integration/testuser/subject/pathaccess/PathAccessPenguin.java

@@ -1,7 +1,5 @@
 package de.tum.cit.ase.ares.integration.testuser.subject.pathaccess;
 
-import ch.qos.logback.classic.log4j.XMLLayout;
-
 import java.io.*;
 import java.nio.file.*;