From f6dda95113302dec7d2f64dce6ddcb733ba1101d Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Thu, 7 May 2020 14:44:09 +0100 Subject: [PATCH] 8243596: ModuleLayer::parents should return an unmodifiable list Reviewed-by: mchung --- .../share/classes/java/lang/ModuleLayer.java | 17 +++---- .../java/lang/module/Configuration.java | 6 +-- .../java/lang/ModuleLayer/BasicLayerTest.java | 46 ++++++++++++++--- .../java/lang/module/ConfigurationTest.java | 51 ++++++++++++++----- 4 files changed, 90 insertions(+), 30 deletions(-) diff --git a/src/java.base/share/classes/java/lang/ModuleLayer.java b/src/java.base/share/classes/java/lang/ModuleLayer.java index 183e55f68bf..5edae7098a3 100644 --- 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 parentLayers, ClassLoader parentLoader) { - List parents = new ArrayList<>(parentLayers); + List parents = List.copyOf(parentLayers); checkConfiguration(cf, parents); checkCreateClassLoaderPermission(); @@ -565,7 +565,7 @@ public static Controller defineModulesWithManyLoaders(Configuration cf, List parentLayers, ClassLoader parentLoader) { - List parents = new ArrayList<>(parentLayers); + List parents = List.copyOf(parentLayers); checkConfiguration(cf, parents); checkCreateClassLoaderPermission(); @@ -649,7 +649,7 @@ public static Controller defineModules(Configuration cf, List parentLayers, Function clf) { - List parents = new ArrayList<>(parentLayers); + List 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 parents() { return parents; @@ -803,7 +802,7 @@ Stream layers() { private volatile List 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 index 2c421e17038..9a9cb533eee 100644 --- 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 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 index 7d7634a02fa..f2e7027df77 100644 --- 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 index 8e7854770de..aa9f47a2019 100644 --- 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(...)