Tidy up container group implementations and classloaders a little

ascopes · ascopes · commit c722740cfa2f · 2022-11-24T14:24:42.000Z
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/compilers/impl/JctFileManagerImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/compilers/impl/JctFileManagerImpl.java
@@ -28,7 +28,6 @@
 import io.github.ascopes.jct.containers.impl.OutputContainerGroupImpl;
 import io.github.ascopes.jct.containers.impl.PackageContainerGroupImpl;
 import io.github.ascopes.jct.pathwrappers.PathWrapper;
-import io.github.ascopes.jct.utils.GarbageDisposalUtils;
 import io.github.ascopes.jct.utils.ToStringBuilder;
 import java.io.IOException;
 import java.lang.module.ModuleFinder;
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/AbstractPackageContainerGroup.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/AbstractPackageContainerGroup.java
@@ -15,6 +15,7 @@
  */
 package io.github.ascopes.jct.containers.impl;
 
+import static java.util.Collections.synchronizedSet;
 import static java.util.Objects.requireNonNull;
 
 import io.github.ascopes.jct.compilers.PathFileObject;
@@ -67,7 +68,7 @@ public abstract class AbstractPackageContainerGroup implements PackageContainerG
 
   // Use a linked hash set here to deduplicate while retaining order. This is an optimisation
   // since defining the same path more than once does not make sense anyway.
-  protected final LinkedHashSet<Container> containers;
+  protected final Set<Container> containers;
   protected final Lazy<ClassLoader> classLoaderLazy;
 
   /**
@@ -80,7 +81,7 @@ protected AbstractPackageContainerGroup(Location location, String release) {
     this.location = requireNonNull(location, "location");
     this.release = requireNonNull(release, "release");
 
-    containers = new LinkedHashSet<>();
+    containers = synchronizedSet(new LinkedHashSet<>());
     classLoaderLazy = new Lazy<>(this::createClassLoader);
   }
 
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupUrlClassLoader.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupUrlClassLoader.java
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ModuleContainerGroupImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ModuleContainerGroupImpl.java
@@ -29,11 +29,11 @@
 import java.lang.module.ModuleFinder;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nullable;
 import javax.annotation.WillCloseWhenClosed;
 import javax.tools.JavaFileManager.Location;
@@ -81,7 +81,7 @@ public ModuleContainerGroupImpl(Location location, String release) {
       );
     }
 
-    modules = new HashMap<>();
+    modules = new ConcurrentHashMap<>();
   }
 
   @Override
@@ -137,6 +137,7 @@ public List<Set<Location>> getLocationsForModules() {
   }
 
   @Nullable
+  @Override
   public PackageContainerGroup getModule(String name) {
     if (name.isEmpty()) {
       throw new IllegalArgumentException("Cannot have module sources with no valid module name");
@@ -209,7 +210,7 @@ private ModulePackageContainerGroupImpl(ModuleLocation location, String release)
 
     @Override
     protected ClassLoader createClassLoader() {
-      return ContainerGroupUrlClassLoader.createClassLoaderFor(this);
+      return new PackageContainerGroupUrlClassLoader(this);
     }
   }
 }
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java
@@ -48,7 +48,7 @@
  * @since 0.0.1
  */
 @API(since = "0.0.1", status = Status.INTERNAL)
-public class OutputContainerGroupImpl
+public final class OutputContainerGroupImpl
     extends AbstractPackageContainerGroup
     implements OutputContainerGroup {
 
@@ -123,7 +123,7 @@ public PathFileObject getFileForInput(String packageName, String relativeName) {
     var moduleHandle = ModuleHandle.tryExtract(packageName);
 
     if (moduleHandle != null) {
-      var module = getOrCreateModule(moduleHandle.getModuleName());
+      var module = getModule(moduleHandle.getModuleName());
 
       if (module != null) {
         var file = module.getFileForInput(moduleHandle.getRest(), relativeName);
@@ -144,13 +144,10 @@ public PathFileObject getFileForOutput(String packageName, String relativeName)
 
     if (moduleHandle != null) {
       var module = getOrCreateModule(moduleHandle.getModuleName());
+      var file = module.getFileForOutput(moduleHandle.getRest(), relativeName);
 
-      if (module != null) {
-        var file = module.getFileForOutput(moduleHandle.getRest(), relativeName);
-
-        if (file != null) {
-          return file;
-        }
+      if (file != null) {
+        return file;
       }
     }
 
@@ -163,7 +160,7 @@ public PathFileObject getJavaFileForInput(String className, Kind kind) {
     var moduleHandle = ModuleHandle.tryExtract(className);
 
     if (moduleHandle != null) {
-      var module = getOrCreateModule(moduleHandle.getModuleName());
+      var module = getModule(moduleHandle.getModuleName());
 
       if (module != null) {
         var file = module.getJavaFileForInput(moduleHandle.getRest(), kind);
@@ -184,13 +181,10 @@ public PathFileObject getJavaFileForOutput(String className, Kind kind) {
 
     if (moduleHandle != null) {
       var module = getOrCreateModule(moduleHandle.getModuleName());
+      var file = module.getJavaFileForOutput(moduleHandle.getRest(), kind);
 
-      if (module != null) {
-        var file = module.getJavaFileForOutput(moduleHandle.getRest(), kind);
-
-        if (file != null) {
-          return file;
-        }
+      if (file != null) {
+        return file;
       }
     }
 
@@ -224,7 +218,7 @@ public boolean hasLocation(ModuleLocation location) {
 
   @Override
   protected ClassLoader createClassLoader() {
-    return ContainerGroupUrlClassLoader.createClassLoaderFor(this);
+    return new PackageContainerGroupUrlClassLoader(this);
   }
 
   @SuppressWarnings("resource")
@@ -258,7 +252,7 @@ private OutputPackageContainerGroupImpl(Location location, String release) {
 
     @Override
     protected ClassLoader createClassLoader() {
-      return ContainerGroupUrlClassLoader.createClassLoaderFor(this);
+      return new PackageContainerGroupUrlClassLoader(this);
     }
   }
 }
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/PackageContainerGroupImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/PackageContainerGroupImpl.java
@@ -31,7 +31,7 @@
  * @since 0.0.1
  */
 @API(since = "0.0.1", status = Status.INTERNAL)
-public class PackageContainerGroupImpl extends AbstractPackageContainerGroup {
+public final class PackageContainerGroupImpl extends AbstractPackageContainerGroup {
 
   /**
    * Initialize this group.
@@ -62,6 +62,6 @@ public PackageContainerGroupImpl(Location location, String release) {
 
   @Override
   protected ClassLoader createClassLoader() {
-    return ContainerGroupUrlClassLoader.createClassLoaderFor(this);
+    return new PackageContainerGroupUrlClassLoader(this);
   }
 }
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/PackageContainerGroupUrlClassLoader.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/PackageContainerGroupUrlClassLoader.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2022 - 2022 Ashley Scopes
+ *
+ * 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
+ *
+ *    http://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 io.github.ascopes.jct.containers.impl;
+
+import io.github.ascopes.jct.containers.Container;
+import io.github.ascopes.jct.containers.PackageContainerGroup;
+import io.github.ascopes.jct.pathwrappers.PathWrapper;
+import java.net.URL;
+import java.net.URLClassLoader;
+import org.apiguardian.api.API;
+import org.apiguardian.api.API.Status;
+
+/**
+ * An extension of the Java {@link URLClassLoader} that wraps around container groups.
+ *
+ * @author Ashley Scopes
+ * @since 0.0.1
+ */
+@API(since = "0.0.1", status = Status.INTERNAL)
+public final class PackageContainerGroupUrlClassLoader extends URLClassLoader {
+
+  /**
+   * Initialise this class loader.
+   *
+   * @param group the container group to use.
+   */
+  public PackageContainerGroupUrlClassLoader(PackageContainerGroup group) {
+    super(
+        "Packages within " + group.getLocation().getName(),
+        group
+            .getPackages()
+            .stream()
+            .map(Container::getPathWrapper)
+            .map(PathWrapper::getUrl)
+            .distinct()
+            .toArray(URL[]::new),
+        ClassLoader.getSystemClassLoader()
+    );
+  }
+}
diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/PathWrappingContainerImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/PathWrappingContainerImpl.java
@@ -48,7 +48,7 @@
  * @since 0.0.1
  */
 @API(since = "0.0.1", status = Status.INTERNAL)
-public class PathWrappingContainerImpl implements Container {
+public final class PathWrappingContainerImpl implements Container {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(PathWrappingContainerImpl.class);
 

Original file line number	Diff line number	Diff line change
`@@ -29,11 +29,11 @@`
`29`	`29`	`import java.lang.module.ModuleFinder;`
`30`	`30`	`import java.util.ArrayList;`
`31`	`31`	`import java.util.Collections;`
`32`		`-import java.util.HashMap;`
`33`	`32`	`import java.util.List;`
`34`	`33`	`import java.util.Map;`
`35`	`34`	`import java.util.ServiceLoader;`
`36`	`35`	`import java.util.Set;`
	`36`	`+import java.util.concurrent.ConcurrentHashMap;`
`37`	`37`	`import javax.annotation.Nullable;`
`38`	`38`	`import javax.annotation.WillCloseWhenClosed;`
`39`	`39`	`import javax.tools.JavaFileManager.Location;`
`@@ -81,7 +81,7 @@ public ModuleContainerGroupImpl(Location location, String release) {`
`81`	`81`	`);`
`82`	`82`	`}`
`83`	`83`
`84`		`- modules = new HashMap<>();`
	`84`	`+ modules = new ConcurrentHashMap<>();`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`@Override`
`@@ -137,6 +137,7 @@ public List<Set<Location>> getLocationsForModules() {`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`@Nullable`
	`140`	`+ @Override`
`140`	`141`	`public PackageContainerGroup getModule(String name) {`
`141`	`142`	`if (name.isEmpty()) {`
`142`	`143`	`throw new IllegalArgumentException("Cannot have module sources with no valid module name");`
`@@ -209,7 +210,7 @@ private ModulePackageContainerGroupImpl(ModuleLocation location, String release)`
`209`	`210`
`210`	`211`	`@Override`
`211`	`212`	`protected ClassLoader createClassLoader() {`
`212`		`- return ContainerGroupUrlClassLoader.createClassLoaderFor(this);`
	`213`	`+ return new PackageContainerGroupUrlClassLoader(this);`
`213`	`214`	`}`
`214`	`215`	`}`
`215`	`216`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@`
`31`	`31`	`* @since 0.0.1`
`32`	`32`	`*/`
`33`	`33`	`@API(since = "0.0.1", status = Status.INTERNAL)`
`34`		`-public class PackageContainerGroupImpl extends AbstractPackageContainerGroup {`
	`34`	`+public final class PackageContainerGroupImpl extends AbstractPackageContainerGroup {`
`35`	`35`
`36`	`36`	`/**`
`37`	`37`	`* Initialize this group.`
`@@ -62,6 +62,6 @@ public PackageContainerGroupImpl(Location location, String release) {`
`62`	`62`
`63`	`63`	`@Override`
`64`	`64`	`protected ClassLoader createClassLoader() {`
`65`		`- return ContainerGroupUrlClassLoader.createClassLoaderFor(this);`
	`65`	`+ return new PackageContainerGroupUrlClassLoader(this);`
`66`	`66`	`}`
`67`	`67`	`}`