Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMykola Nikishov2019-05-19 18:42:09 -0400
committerMickael Istria2019-08-07 03:37:01 -0400
commitfdaa0214fab1137233cdf143fb63cc1a51db8ef5 (patch)
treeaf279040fa515d19e7732686c3ce86731bafc6de
parent8f540ec08cb091dc58b1e4b851faac56fb009f2f (diff)
downloadrt.equinox.p2-fdaa0214fab1137233cdf143fb63cc1a51db8ef5.tar.gz
rt.equinox.p2-fdaa0214fab1137233cdf143fb63cc1a51db8ef5.tar.xz
rt.equinox.p2-fdaa0214fab1137233cdf143fb63cc1a51db8ef5.zip
Bug 547461 - Make all ProvidedCapability's ctors use immutable mapsI20190807-1800
ProvidedCapability(String, Map<String, Object>): - (good) stores properties in an unmodifiable map - (bad) properties still could be changed via original map - (bad) would not create an empty version if missing - (bad) holds a reference to the original map ProvidedCapability(String, String, Version): - (bad) stores properties in a mutable map Always store properties as unmodifiable map to prevent their mutation via getProperties().put(). As query operations on that map are "read through" to the original map, first make a copy of original properties. Change-Id: If7cf15d153d72e4bedc30e9ad52423d5121f3fa2 Signed-off-by: Mykola Nikishov <mn@mn.com.ua>
-rw-r--r--bundles/org.eclipse.equinox.p2.metadata/src/org/eclipse/equinox/internal/p2/metadata/ProvidedCapability.java12
-rw-r--r--bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/metadata/ProvidedCapabilityTest.java65
2 files changed, 71 insertions, 6 deletions
diff --git a/bundles/org.eclipse.equinox.p2.metadata/src/org/eclipse/equinox/internal/p2/metadata/ProvidedCapability.java b/bundles/org.eclipse.equinox.p2.metadata/src/org/eclipse/equinox/internal/p2/metadata/ProvidedCapability.java
index 5b22d2960..9c251928d 100644
--- a/bundles/org.eclipse.equinox.p2.metadata/src/org/eclipse/equinox/internal/p2/metadata/ProvidedCapability.java
+++ b/bundles/org.eclipse.equinox.p2.metadata/src/org/eclipse/equinox/internal/p2/metadata/ProvidedCapability.java
@@ -61,21 +61,23 @@ public class ProvidedCapability implements IProvidedCapability, IMemberProvider
// Verify the version
Object version = resolvedProps.get(PROPERTY_VERSION);
if (version != null) {
- Assert.isTrue(props.get(PROPERTY_VERSION) instanceof Version);
+ Assert.isTrue(version instanceof Version);
} else {
resolvedProps.put(PROPERTY_VERSION, Version.emptyVersion);
}
- this.properties = Collections.unmodifiableMap(props);
+ this.properties = Collections.unmodifiableMap(resolvedProps);
}
public ProvidedCapability(String namespace, String name, Version version) {
Assert.isNotNull(namespace, NLS.bind(Messages.provided_capability_namespace_not_defined, null));
Assert.isNotNull(name, NLS.bind(Messages.provided_capability_name_not_defined, namespace));
this.namespace = namespace;
- this.properties = new HashMap<>();
- properties.put(namespace, name);
- properties.put(PROPERTY_VERSION, version == null ? Version.emptyVersion : version);
+
+ Map<String, Object> props = new HashMap<>();
+ props.put(namespace, name);
+ props.put(PROPERTY_VERSION, version == null ? Version.emptyVersion : version);
+ this.properties = Collections.unmodifiableMap(props);
}
@Override
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/metadata/ProvidedCapabilityTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/metadata/ProvidedCapabilityTest.java
index d1478c731..44875a623 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/metadata/ProvidedCapabilityTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/metadata/ProvidedCapabilityTest.java
@@ -13,7 +13,11 @@
*******************************************************************************/
package org.eclipse.equinox.p2.tests.metadata;
-import org.eclipse.equinox.p2.metadata.*;
+import java.util.HashMap;
+import java.util.Map;
+import org.eclipse.equinox.p2.metadata.IProvidedCapability;
+import org.eclipse.equinox.p2.metadata.MetadataFactory;
+import org.eclipse.equinox.p2.metadata.Version;
import org.eclipse.equinox.p2.tests.AbstractProvisioningTest;
/**
@@ -28,4 +32,63 @@ public class ProvidedCapabilityTest extends AbstractProvisioningTest {
assertFalse("1.1", cap.equals(notEqual));
assertFalse("1.1", notEqual.equals(cap));
}
+
+ public void testProperties_Unmodifiable() {
+ String namespace = "aNamespace";
+ String name = "name";
+ Version version = Version.createOSGi(2, 0, 0);
+
+ Map properties = new HashMap<>();
+ properties.put(namespace, name);
+ properties.put(IProvidedCapability.PROPERTY_VERSION, version);
+
+ IProvidedCapability capability1 = MetadataFactory.createProvidedCapability(namespace, properties);
+ IProvidedCapability capability2 = MetadataFactory.createProvidedCapability(namespace, name, version);
+ assertEquals(capability1, capability2);
+
+ try {
+ capability1.getProperties().put("key", "value");
+ fail("properties must be unmodifiable");
+ } catch (UnsupportedOperationException e) {
+ // ok
+ }
+
+ try {
+ capability2.getProperties().put("key", "value");
+ fail("properties must be unmodifiable");
+ } catch (UnsupportedOperationException e) {
+ // ok
+ }
+ }
+
+ public void testProperties_Immutable() {
+ String namespace = "aNamespace";
+ String name = "name";
+ Version version = Version.createOSGi(2, 0, 0);
+
+ Map properties = new HashMap<>();
+ properties.put(namespace, name);
+ properties.put(IProvidedCapability.PROPERTY_VERSION, version);
+
+ IProvidedCapability capability1 = MetadataFactory.createProvidedCapability(namespace, properties);
+ IProvidedCapability capability2 = MetadataFactory.createProvidedCapability(namespace, name, version);
+
+ // mutate original value
+ properties.put(IProvidedCapability.PROPERTY_VERSION, Version.createOSGi(9, 9, 9));
+
+ assertEquals(capability1, capability2);
+ }
+
+ public void testProperties_NoVersion() {
+ String namespace = "aNamespace";
+ String name = "name";
+
+ Map properties = new HashMap<>();
+ properties.put(namespace, name);
+ // no version this time
+
+ IProvidedCapability capability1 = MetadataFactory.createProvidedCapability(namespace, properties);
+ IProvidedCapability capability2 = MetadataFactory.createProvidedCapability(namespace, name, null);
+ assertEquals(capability1, capability2);
+ }
}

Back to the top