8243596: ModuleLayer::parents should return an unmodifiable list

Reviewed-by: mchung
openjdk · May 7, 2020 · f6dda95 · f6dda95
1 parent 93b0516
commit f6dda95
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 30 deletions.
diff --git a/src/java.base/share/classes/java/lang/ModuleLayer.java b/src/java.base/share/classes/java/lang/ModuleLayer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -489,7 +489,7 @@ public static Controller defineModulesWithOneLoader(Configuration cf,
                                                         List<ModuleLayer> parentLayers,
                                                         ClassLoader parentLoader)
     {
-        List<ModuleLayer> parents = new ArrayList<>(parentLayers);
+        List<ModuleLayer> parents = List.copyOf(parentLayers);
         checkConfiguration(cf, parents);
 
         checkCreateClassLoaderPermission();
@@ -565,7 +565,7 @@ public static Controller defineModulesWithManyLoaders(Configuration cf,
                                                           List<ModuleLayer> parentLayers,
                                                           ClassLoader parentLoader)
     {
-        List<ModuleLayer> parents = new ArrayList<>(parentLayers);
+        List<ModuleLayer> parents = List.copyOf(parentLayers);
         checkConfiguration(cf, parents);
 
         checkCreateClassLoaderPermission();
@@ -649,7 +649,7 @@ public static Controller defineModules(Configuration cf,
                                            List<ModuleLayer> parentLayers,
                                            Function<String, ClassLoader> clf)
     {
-        List<ModuleLayer> parents = new ArrayList<>(parentLayers);
+        List<ModuleLayer> parents = List.copyOf(parentLayers);
         checkConfiguration(cf, parents);
         Objects.requireNonNull(clf);
 
@@ -752,13 +752,12 @@ public Configuration configuration() {
         return cf;
     }
 
-
     /**
-     * Returns the list of this layer's parents unless this is the
-     * {@linkplain #empty empty layer}, which has no parents and so an
+     * Returns an unmodifiable list of this layer's parents, in search
+     * order. If this is the {@linkplain #empty() empty layer} then an
      * empty list is returned.
      *
-     * @return The list of this layer's parents
+     * @return A possibly-empty unmodifiable list of this layer's parents
      */
     public List<ModuleLayer> parents() {
         return parents;
@@ -803,7 +802,7 @@ Stream<ModuleLayer> layers() {
     private volatile List<ModuleLayer> allLayers;
 
     /**
-     * Returns the set of the modules in this layer.
+     * Returns an unmodifiable set of the modules in this layer.
      *
      * @return A possibly-empty unmodifiable set of the modules in this layer
      */

diff --git a/src/java.base/share/classes/java/lang/module/Configuration.java b/src/java.base/share/classes/java/lang/module/Configuration.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -509,7 +509,7 @@ public static Configuration empty() {
 
     /**
      * Returns an unmodifiable list of this configuration's parents, in search
-     * order. If this is the {@linkplain #empty empty configuration} then an
+     * order. If this is the {@linkplain #empty() empty configuration} then an
      * empty list is returned.
      *
      * @return A possibly-empty unmodifiable list of this parent configurations
@@ -520,7 +520,7 @@ public List<Configuration> parents() {
 
 
     /**
-     * Returns an immutable set of the resolved modules in this configuration.
+     * Returns an unmodifiable set of the resolved modules in this configuration.
      *
      * @return A possibly-empty unmodifiable set of the resolved modules
      *         in this configuration

diff --git a/test/jdk/java/lang/ModuleLayer/BasicLayerTest.java b/test/jdk/java/lang/ModuleLayer/BasicLayerTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1195,14 +1195,48 @@ public void testFindLoaderWithNull() {
     }
 
 
-    // immutable sets
+    // unmodifiable collections
 
-    @Test(expectedExceptions = { UnsupportedOperationException.class })
-    public void testImmutableSet() {
-        Module base = Object.class.getModule();
-        ModuleLayer.boot().modules().add(base);
+    @DataProvider(name = "layers")
+    public Object[][] layers() {
+        Configuration cf = resolve(ModuleFinder.of());
+        ModuleLayer layer1 = ModuleLayer.empty().defineModulesWithOneLoader(cf, null);
+        ModuleLayer layer2 = ModuleLayer.empty().defineModulesWithManyLoaders(cf, null);
+        ModuleLayer layer3 = ModuleLayer.empty().defineModules(cf, mn -> null);
+
+        // empty, boot, and custom layers
+        return new Object[][] {
+            { ModuleLayer.empty(), null },
+            { ModuleLayer.boot(),  null },
+            { layer1,              null },
+            { layer2,              null },
+            { layer3,              null },
+        };
     }
 
+    @Test(dataProvider = "layers",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableParents1(ModuleLayer layer, Object ignore) {
+        layer.parents().add(ModuleLayer.empty());
+    }
+
+    @Test(dataProvider = "layers",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableParents2(ModuleLayer layer, Object ignore) {
+        layer.parents().remove(ModuleLayer.empty());
+    }
+
+    @Test(dataProvider = "layers",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableModules1(ModuleLayer layer, Object ignore) {
+        layer.modules().add(Object.class.getModule());
+    }
+
+    @Test(dataProvider = "layers",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableModules2(ModuleLayer layer, Object ignore) {
+        layer.modules().remove(Object.class.getModule());
+    }
 
     /**
      * Resolve the given modules, by name, and returns the resulting

diff --git a/test/jdk/java/lang/module/ConfigurationTest.java b/test/jdk/java/lang/module/ConfigurationTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -2033,22 +2033,49 @@ public void testFindModuleWithNull() {
         Configuration.empty().findModule(null);
     }
 
-    // immutable sets
+    // unmodifiable collections
 
-    @Test(expectedExceptions = { UnsupportedOperationException.class })
-    public void testImmutableSet1() {
-        Configuration cf = ModuleLayer.boot().configuration();
-        ResolvedModule base = cf.findModule("java.base").get();
-        cf.modules().add(base);
+    @DataProvider(name = "configurations")
+    public Object[][] configurations() {
+        // empty, boot, and custom configurations
+        return new Object[][] {
+            { Configuration.empty(),              null },
+            { ModuleLayer.boot().configuration(), null },
+            { resolve(ModuleFinder.of()),         null },
+        };
     }
 
-    @Test(expectedExceptions = { UnsupportedOperationException.class })
-    public void testImmutableSet2() {
-        Configuration cf = ModuleLayer.boot().configuration();
-        ResolvedModule base = cf.findModule("java.base").get();
-        base.reads().add(base);
+    @Test(dataProvider = "configurations",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableParents1(Configuration cf, Object ignore) {
+        cf.parents().add(Configuration.empty());
     }
 
+    @Test(dataProvider = "configurations",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableParents2(Configuration cf, Object ignore) {
+        cf.parents().remove(Configuration.empty());
+    }
+
+    @Test(dataProvider = "configurations",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableModules1(Configuration cf, Object ignore) {
+        ResolvedModule module = ModuleLayer.boot()
+                .configuration()
+                .findModule("java.base")
+                .orElseThrow();
+        cf.modules().add(module);
+    }
+
+    @Test(dataProvider = "configurations",
+            expectedExceptions = { UnsupportedOperationException.class })
+    public void testUnmodifiableModules2(Configuration cf, Object ignore) {
+        ResolvedModule module = ModuleLayer.boot()
+                .configuration()
+                .findModule("java.base")
+                .orElseThrow();
+        cf.modules().remove(module);
+    }
 
     /**
      * Invokes parent.resolve(...)