Fix FilterImpl.WrapperCapability: Filter.matches(Map) always throws

.github/workflows/maven-ci.yml

@@ -86,7 +86,11 @@ jobs:
       run: mvn -B -V -Dstyle.color=always --file webconsole/pom.xml clean install verify
     - name: Felix Framework
       if: steps.changes.outputs.framework == 'true'
-      run: mvn -B -V -Dstyle.color=always --file framework/pom.xml clean verify
+      # install, not verify: the TCK below is a separate Maven invocation and resolves
+      # the framework from the repository, so without installing it silently tests
+      # whatever org.apache.felix.framework is published rather than the build under
+      # test.
+      run: mvn -B -V -Dstyle.color=always --file framework/pom.xml clean install
     - name: OSGi-TCK Framework
       if: steps.changes.outputs.framework == 'true'
       run: mvn -B -V -Dstyle.color=always --file framework.tck/pom.xml clean verify

framework.tck/pom.xml

@@ -105,7 +105,7 @@
     <dependency>
       <groupId>net.bytebuddy</groupId>
       <artifactId>byte-buddy</artifactId>
-      <version>1.17.5</version>
+      <version>1.18.12</version>
       <scope>test</scope>
     </dependency>
     <dependency>
@@ -120,12 +120,6 @@
       <version>4.13.2_1</version>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.junit.platform</groupId>
-      <artifactId>junit-platform-launcher</artifactId>
-      <version>1.12.1</version>
-      <scope>test</scope>
-    </dependency>
     <dependency>
       <groupId>org.junit.platform</groupId>
       <artifactId>junit-platform-engine</artifactId>

framework.tck/tck.bndrun

@@ -30,7 +30,7 @@
 	junit-platform-engine;version='[1.12.1,1.12.2)',\
 	org.opentest4j;version='[1.3.0,1.3.1)',\
 	junit-platform-launcher;version='[1.12.1,1.12.2)',\
-	assertj-core;version='[3.27.3,3.27.4)',\
+	assertj-core;version='[3.27.7,3.27.8)',\
 	biz.aQute.junit;version='[6.4.1,6.4.2)',\
 	junit-vintage-engine;version='[5.7.1,5.7.2)',\
-	net.bytebuddy.byte-buddy;version='[1.17.5,1.17.6)'
+	net.bytebuddy.byte-buddy;version='[1.18.12,1.18.13)'

framework/src/main/java/org/apache/felix/framework/FilterImpl.java

@@ -104,13 +104,11 @@ static class WrapperCapability extends BundleCapabilityImpl
     {
         private final Map<String, Object> m_map;
 
+        @SuppressWarnings("unchecked")
         public WrapperCapability(Map<String, ?> map)
         {
             super(null, null, Collections.emptyMap(), Collections.emptyMap());
-                m_map = Collections.emptyMap();
-                if(map != null ) {
-            }
-            m_map.putAll(map);
+            m_map = (map == null) ? Collections.emptyMap() : (Map<String, Object>) map;
         }
 
         public WrapperCapability(Dictionary<String, ?> dict, boolean caseSensitive)
@@ -122,7 +120,14 @@ public WrapperCapability(Dictionary<String, ?> dict, boolean caseSensitive)
         public WrapperCapability(ServiceReference<?> sr)
         {
             super(null, null, Collections.emptyMap(), Collections.emptyMap());
-            m_map = new DictionaryToMap(sr.getProperties(), false);
+            // Read the properties one by one rather than via getProperties(): that
+            // method was only added in OSGi Core 1.10 and is not implemented by every
+            // ServiceReference, whereas getPropertyKeys()/getProperty() always are.
+            m_map = new StringMap();
+            for (String key : sr.getPropertyKeys())
+            {
+                m_map.put(key, sr.getProperty(key));
+            }
         }
 
         @Override

framework/src/test/java/org/apache/felix/framework/FilterTest.java

@@ -23,10 +23,12 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Dictionary;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import org.junit.jupiter.api.Test;
@@ -104,4 +106,36 @@ private static Dictionary<String, Object> createTestDict(Object o)
         return dictionary;
     }
 
+    /**
+     * Filter.matches(Map) used to throw UnsupportedOperationException for any
+     * non-empty map, because WrapperCapability assigned an immutable empty map and
+     * then called putAll on it. See FELIX-6759 discussion; regression from
+     * 466eb93f1c.
+     */
+    @Test
+    void matchesNonEmptyMap() throws InvalidSyntaxException
+    {
+        Filter filter = new FilterImpl("(one=one-value)");
+
+        Map<String, Object> map = new HashMap<>();
+        map.put("one", "one-value");
+
+        assertThat(filter.matches(map)).isTrue();
+        assertThat(filter.matches(Collections.singletonMap("one", "other-value"))).isFalse();
+    }
+
+    /**
+     * The same constructor threw NullPointerException for a null map, where it used
+     * to fall back to an empty one.
+     */
+    @Test
+    void matchesEmptyAndUnmodifiableMap() throws InvalidSyntaxException
+    {
+        Filter filter = new FilterImpl("(one=one-value)");
+
+        assertThat(filter.matches(Collections.<String, Object>emptyMap())).isFalse();
+        assertThat(filter.matches(
+            Collections.unmodifiableMap(Collections.singletonMap("one", (Object) "one-value")))).isTrue();
+    }
+
 }