You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by da...@apache.org on 2021/08/30 13:23:07 UTC

[felix-dev] branch master updated: Fixes for gaps identified by the TCK

This is an automated email from the ASF dual-hosted git repository.

davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git


The following commit(s) were added to refs/heads/master by this push:
     new 4431984  Fixes for gaps identified by the TCK
     new 9abf655  Merge pull request #93 from bosschaert/tck_fixes
4431984 is described below

commit 44319843f495e7bbfc1fef70cc85ebe835326cb8
Author: David Bosschaert <da...@apache.org>
AuthorDate: Fri Aug 27 15:32:05 2021 +0100

    Fixes for gaps identified by the TCK
---
 .../felix/feature/impl/ArtifactBuilderImpl.java    |  23 ++++-
 .../felix/feature/impl/BundleBuilderImpl.java      |  21 +++++
 .../felix/feature/impl/FeatureServiceImpl.java     |   8 ++
 .../java/org/apache/felix/feature/impl/IDImpl.java |   7 ++
 .../felix/feature/impl/BundleBuilderImplTest.java  | 105 +++++++++++++++++++++
 5 files changed, 163 insertions(+), 1 deletion(-)

diff --git a/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java b/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java
index 4fe6584..3af5e71 100644
--- a/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java
+++ b/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java
@@ -36,13 +36,34 @@ class ArtifactBuilderImpl implements FeatureArtifactBuilder {
 
     @Override
     public FeatureArtifactBuilder addMetadata(String key, Object value) {
+    	if (key == null)
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+
+    	if (value == null)
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+    	
+    	if ("id".equalsIgnoreCase(key))
+    		throw new IllegalArgumentException("Key cannot be 'id'");
+    	    	
         this.metadata.put(key, value);
         return this;
     }
 
     @Override
     public FeatureArtifactBuilder addMetadata(Map<String,Object> md) {
-        this.metadata.putAll(md);
+    	if (md.keySet().contains(null))
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+    	
+    	if (md.values().contains(null))
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+    	
+    	if (md.keySet().stream()
+    		.map(String::toLowerCase)
+    		.anyMatch(s -> "id".equals(s))) {
+    		throw new IllegalArgumentException("Key cannot be 'id'");    		
+    	}
+
+    	this.metadata.putAll(md);
         return this;
     }
 
diff --git a/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java b/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java
index e19a2b1..1f55d03 100644
--- a/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java
+++ b/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java
@@ -36,12 +36,33 @@ class BundleBuilderImpl implements FeatureBundleBuilder {
 
     @Override
     public FeatureBundleBuilder addMetadata(String key, Object value) {
+    	if (key == null)
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+
+    	if (value == null)
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+    	
+    	if ("id".equalsIgnoreCase(key))
+    		throw new IllegalArgumentException("Key cannot be 'id'");
+    	
         this.metadata.put(key, value);
         return this;
     }
 
     @Override
     public FeatureBundleBuilder addMetadata(Map<String,Object> md) {
+    	if (md.keySet().contains(null))
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+    	
+    	if (md.values().contains(null))
+    		throw new IllegalArgumentException("Metadata key cannot be null");
+    	
+    	if (md.keySet().stream()
+    		.map(String::toLowerCase)
+    		.anyMatch(s -> "id".equals(s))) {
+    		throw new IllegalArgumentException("Key cannot be 'id'");    		
+    	}
+    		    	
         this.metadata.putAll(md);
         return this;
     }
diff --git a/features/src/main/java/org/apache/felix/feature/impl/FeatureServiceImpl.java b/features/src/main/java/org/apache/felix/feature/impl/FeatureServiceImpl.java
index 69d61c0..a25021d 100644
--- a/features/src/main/java/org/apache/felix/feature/impl/FeatureServiceImpl.java
+++ b/features/src/main/java/org/apache/felix/feature/impl/FeatureServiceImpl.java
@@ -78,11 +78,19 @@ public class FeatureServiceImpl implements FeatureService {
 
 	@Override
 	public ID getID(String groupId, String artifactId, String version, String type) {
+		if (type == null)
+			throw new NullPointerException("type must not be null");
+		
 		return new IDImpl(groupId, artifactId, version, type, null);
 	}
 
 	@Override
 	public ID getID(String groupId, String artifactId, String version, String type, String classifier) {
+		if (type == null)
+			throw new NullPointerException("type must not be null");
+		if (classifier == null)
+			throw new NullPointerException("classifier must not be null");
+
 		return new IDImpl(groupId, artifactId, version, type, classifier);
 	}
 
diff --git a/features/src/main/java/org/apache/felix/feature/impl/IDImpl.java b/features/src/main/java/org/apache/felix/feature/impl/IDImpl.java
index 76ef473..be533f5 100644
--- a/features/src/main/java/org/apache/felix/feature/impl/IDImpl.java
+++ b/features/src/main/java/org/apache/felix/feature/impl/IDImpl.java
@@ -41,6 +41,13 @@ public class IDImpl implements ID {
 			throws IllegalArgumentException {
         String[] parts = mavenID.split(":");
 
+        
+        if (mavenID.startsWith(":") 
+        		|| mavenID.endsWith(":") 
+        		|| mavenID.contains("::")) {
+            throw new IllegalArgumentException("Not a valid maven ID" + mavenID);        	
+        }
+        
         if (parts.length < 3 || parts.length > 5)
             throw new IllegalArgumentException("Not a valid maven ID" + mavenID);
 
diff --git a/features/src/test/java/org/apache/felix/feature/impl/BundleBuilderImplTest.java b/features/src/test/java/org/apache/felix/feature/impl/BundleBuilderImplTest.java
new file mode 100644
index 0000000..e71687f
--- /dev/null
+++ b/features/src/test/java/org/apache/felix/feature/impl/BundleBuilderImplTest.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You 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 org.apache.felix.feature.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.osgi.service.feature.FeatureBundle;
+import org.osgi.service.feature.FeatureService;
+import org.osgi.service.feature.ID;
+
+public class BundleBuilderImplTest {
+	private FeatureService featureService;
+	
+	@Before
+	public void setUp() {
+		featureService = new FeatureServiceImpl();
+	}
+	
+	@Test
+	public void testMetadata() {
+		ID id = featureService.getID("g", "a", "v");
+		BundleBuilderImpl bb = new BundleBuilderImpl(id);
+		
+		bb.addMetadata("foo", "bar");
+		
+		try {
+			bb.addMetadata("blah", null);
+			fail();
+		} catch (IllegalArgumentException iae) {
+			// good
+		}
+
+		try {
+			bb.addMetadata(null, "blah");
+			fail();
+		} catch (IllegalArgumentException iae) {
+			// good
+		}
+
+		try {
+			bb.addMetadata("id", "blah");
+			fail();
+		} catch (IllegalArgumentException iae) {
+			// good
+		}
+		
+		FeatureBundle bundle = bb.build();
+		assertEquals("g:a:v", bundle.getID().toString());
+		assertEquals(Collections.singletonMap("foo", "bar"),
+				bundle.getMetadata());
+	}
+	
+	@Test
+	public void testMetadataMap() {
+		ID id = featureService.getID("g", "a", "v", "t", "c");
+		BundleBuilderImpl bb = new BundleBuilderImpl(id);
+		
+		bb.addMetadata(Collections.singletonMap("foo", "bar"));
+		
+		try {
+			bb.addMetadata(Collections.singletonMap("blah", null));
+			fail();
+		} catch (IllegalArgumentException iae) {
+			// good
+		}
+
+		try {
+			bb.addMetadata(Collections.singletonMap(null, "blah"));
+			fail();
+		} catch (IllegalArgumentException iae) {
+			// good
+		}
+
+		try {
+			bb.addMetadata(Collections.singletonMap("id", "blah"));
+			fail();
+		} catch (IllegalArgumentException iae) {
+			// good
+		}
+		
+		FeatureBundle bundle = bb.build();
+		assertEquals("g:a:t:c:v", bundle.getID().toString());
+		assertEquals(Collections.singletonMap("foo", "bar"),
+				bundle.getMetadata());
+	}	
+}