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 <dav...@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()); + } +}