You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by cs...@apache.org on 2013/04/12 17:37:52 UTC

svn commit: r1467313 - in /cxf/dosgi/trunk/dsw/cxf-dsw/src: main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java

Author: cschneider
Date: Fri Apr 12 15:37:52 2013
New Revision: 1467313

URL: http://svn.apache.org/r1467313
Log:
DOSGI-168 Fix handling of service parameters with thanks to Amichai

Modified:
    cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java
    cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java

Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java?rev=1467313&r1=1467312&r2=1467313&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java Fri Apr 12 15:37:52 2013
@@ -18,6 +18,7 @@
  */
 package org.apache.cxf.dosgi.dsw.service;
 
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -78,13 +79,9 @@ public class RemoteServiceAdminCore impl
         if (additionalProperties != null) {
             OsgiUtils.overlayProperties(serviceProperties, additionalProperties);
         }
+        Map<String, Object> key = makeKey(serviceProperties);
 
         List<String> interfaces = getInterfaces(serviceProperties);
-        if (interfaces.size() == 0) {
-            LOG.error("export failed: no provided service interfaces found or service_exported_interfaces is null !!");
-            // TODO: publish error event ? not sure
-            return Collections.emptyList();
-        }
 
         if (isCreatedByThisRSA(serviceReference)) {
             LOG.debug("Skipping export of this service as we created it ourself as a proxy {}", interfaces);
@@ -94,9 +91,10 @@ public class RemoteServiceAdminCore impl
 
         synchronized (exportedServices) {
             // check if it is already exported ....
-            if (exportedServices.containsKey(serviceProperties)) {
+            Collection<ExportRegistration> existingRegs = exportedServices.get(key);
+            if (existingRegs != null) {
                 LOG.debug("already exported this service. Returning existing exportRegs {} ", interfaces);
-                return copyExportRegistration(serviceProperties);
+                return copyExportRegistration(existingRegs);
             }
             LOG.info("interfaces selected for export: " + interfaces);
             List<ExportRegistration> exportRegs = new ArrayList<ExportRegistration>(1);
@@ -133,69 +131,95 @@ public class RemoteServiceAdminCore impl
             }
 
             // enlist initial export Registrations in global list of exportRegistrations
-            exportedServices.put(serviceProperties, new ArrayList<ExportRegistration>(exportRegs));
+            exportedServices.put(key, new ArrayList<ExportRegistration>(exportRegs));
             eventProducer.publishNotifcation(exportRegs);
             return exportRegs;
         }
     }
 
     /**
-     * Determine which interfaces should be exported
+     * Determines which interfaces should be exported.
      *
-     * @param serviceProperties
-     * @return interfaces to be exported
+     * @param serviceProperties the exported service properties
+     * @return the interfaces to be exported
+     * @throws IllegalArgumentException if the service parameters are invalid
+     * @see RemoteServiceAdmin#exportService
+     * @see org.osgi.framework.Constants#OBJECTCLASS
+     * @see RemoteConstants#SERVICE_EXPORTED_INTERFACES
      */
     private List<String> getInterfaces(Map<String, Object> serviceProperties) {
-        List<String> interfaces = new ArrayList<String>(1);
-
-        // Earlier in this class we have converted objectClass from String[] to List<String>...
-        @SuppressWarnings("unchecked")
-        List<String> providedInterfaces = (List<String>) serviceProperties.get(org.osgi.framework.Constants.OBJECTCLASS);
+        String[] providedInterfaces = (String[])serviceProperties.get(org.osgi.framework.Constants.OBJECTCLASS);
+        if (providedInterfaces == null || providedInterfaces.length == 0) {
+            throw new IllegalArgumentException("service is missing the objectClass property");
+        }
 
         String[] allowedInterfaces
             = Utils.normalizeStringPlus(serviceProperties.get(RemoteConstants.SERVICE_EXPORTED_INTERFACES));
-        if (providedInterfaces == null || allowedInterfaces == null) {
-            return Collections.emptyList();
+        if (allowedInterfaces == null || allowedInterfaces.length == 0) {
+            throw new IllegalArgumentException("service is missing the service.exported.interfaces property");
         }
 
+        List<String> interfaces = new ArrayList<String>(1);
         if (allowedInterfaces.length == 1 && "*".equals(allowedInterfaces[0])) {
-            for (String i : providedInterfaces) {
-                interfaces.add(i);
-            }
+            // FIXME: according to the spec, this should only return the interfaces, and not
+            // non-interface classes (which are valid OBJECTCLASS values, even if discouraged)
+            Collections.addAll(interfaces, providedInterfaces);
         } else {
-            for (String x : allowedInterfaces) {
-                for (String i : providedInterfaces) {
-                    if (x.equals(i)) {
-                        interfaces.add(i);
-                    }
-                }
+            List<String> providedList = Arrays.asList(providedInterfaces);
+            List<String> allowedList = Arrays.asList(allowedInterfaces);
+            if (!providedList.containsAll(allowedList)) {
+                throw new IllegalArgumentException(String.format(
+                    "exported interfaces %s must be a subset of the service's registered types %s",
+                    allowedList, providedList));
             }
+
+            Collections.addAll(interfaces, allowedInterfaces);
         }
         return interfaces;
     }
 
+    /**
+     * Returns a service's properties as a map.
+     *
+     * @param serviceReference a service reference
+     * @return the service's properties as a map
+     */
     private Map<String, Object> getProperties(ServiceReference serviceReference) {
-        Map<String, Object> serviceProperties = new HashMap<String, Object>();
-        for (String key : serviceReference.getPropertyKeys()) {
+        String[] keys = serviceReference.getPropertyKeys();
+        Map<String, Object> props = new HashMap<String, Object>(keys.length);
+        for (String key : keys) {
             Object val = serviceReference.getProperty(key);
-            if (val instanceof Object[]) {
-                // If we are dealing with an array, turn it into a list because otherwise the equals method on the map key won't work
-                Object[] arr = (Object[]) val;
-                List<Object> l = new ArrayList<Object>();
-                for (Object o : arr)
-                    l.add(o);
+            props.put(key, val);
+        }
+        return props;
+    }
 
-                serviceProperties.put(key, l);
-            } else {
-                serviceProperties.put(key, val);
+    /**
+     * Converts the given properties map into one that can be used as a map key itself.
+     * For example, if a value is an array, it is converted into a list so that the
+     * equals method will compare it properly.
+     *
+     * @param properties a properties map
+     * @return a map that represents the given map, but can be safely used as a map key itself
+     */
+    private Map<String, Object> makeKey(Map<String, Object> properties) {
+        // FIXME: we should also make logically equal values actually compare as equal
+        // (e.g. String+ values should be normalized)
+        Map<String, Object> converted = new HashMap<String, Object>(properties.size());
+        for (Map.Entry<String, Object> entry : properties.entrySet()) {
+            Object val = entry.getValue();
+            if (val instanceof Object[]) {
+                Object[] arr = (Object[])val;
+                List<Object> list = new ArrayList<Object>(arr.length);
+                Collections.addAll(list, arr);
+                val = list;
             }
+            converted.put(entry.getKey(), val);
         }
-        return serviceProperties;
+        return converted;
     }
 
-    private List<ExportRegistration> copyExportRegistration(Map<String, Object> serviceProperties) {
-        Collection<ExportRegistration> regs = exportedServices.get(serviceProperties);
-
+    private List<ExportRegistration> copyExportRegistration(Collection<ExportRegistration> regs) {
         List<EndpointDescription> copiedEndpoints = new ArrayList<EndpointDescription>();
 
         // / create a new list with copies of the exportRegistrations
@@ -220,6 +244,7 @@ public class RemoteServiceAdminCore impl
 
 
     private boolean isCreatedByThisRSA(ServiceReference sref) {
+        Bundle serviceBundle = sref.getBundle();
         return (sref.getBundle() != null) && sref.getBundle().equals(bctx.getBundle());
     }
 

Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java?rev=1467313&r1=1467312&r2=1467313&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java Fri Apr 12 15:37:52 2013
@@ -27,7 +27,6 @@ import static org.junit.Assert.assertTru
 import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.Hashtable;
@@ -52,6 +51,7 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.Constants;
 import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.ServiceListener;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
 import org.osgi.service.remoteserviceadmin.ExportRegistration;
@@ -169,13 +169,14 @@ public class RemoteServiceAdminCoreTest 
 
     @Test
     public void testExport() throws Exception {
-        BundleContext bc = EasyMock.createNiceMock(BundleContext.class);
+        BundleContext bc = EasyMock.createMock(BundleContext.class);
+        EasyMock.expect(bc.getProperty(Constants.FRAMEWORK_VERSION)).andReturn(null).anyTimes();
+        bc.addServiceListener(EasyMock.<ServiceListener>anyObject(), EasyMock.<String>anyObject());
+        EasyMock.expectLastCall().anyTimes();
+        EasyMock.expect(bc.getServiceReferences(EasyMock.<String>anyObject(), EasyMock.<String>anyObject())).andReturn(null).anyTimes();
+        EasyMock.expect(bc.getAllServiceReferences(EasyMock.<String>anyObject(), EasyMock.<String>anyObject())).andReturn(null).anyTimes();
 
-        Bundle b = EasyMock.createNiceMock(Bundle.class);
-        EasyMock.expect(b.getBundleContext()).andReturn(bc).anyTimes();
-        EasyMock.expect(b.getSymbolicName()).andReturn("rsabundle").anyTimes();
-        EasyMock.expect(b.getHeaders()).andReturn(new Hashtable<String, String>()).anyTimes();
-        EasyMock.replay(b);
+        Bundle b = createDummyRsaBundle(bc);
 
         final Map<String, Object> sProps = new HashMap<String, Object>();
         sProps.put("objectClass", new String[] {"java.lang.Runnable"});
@@ -192,25 +193,17 @@ public class RemoteServiceAdminCoreTest 
         EasyMock.expect(bc.createFilter("(service.id=51)")).andReturn(FrameworkUtil.createFilter("(service.id=51)")).anyTimes();
         EasyMock.replay(bc);
 
-        // The service properties but with Arrays converted into lists (which is what the impl internally uses
-        // to allow these things to be compared).
-        Map<String, Object> sPropsMod = new HashMap<String, Object>();
-        sPropsMod.put("objectClass", Collections.singletonList("java.lang.Runnable"));
-        sPropsMod.put("service.id", 51L);
-        sPropsMod.put("myProp", "myVal");
-        sPropsMod.put("service.exported.interfaces", "*");
-
         HashMap<String, Object> eProps = new HashMap<String, Object>(sProps);
         eProps.put("endpoint.id", "http://something");
         eProps.put("service.imported.configs", new String[] {"org.apache.cxf.ws"});
         ExportResult er = new ExportResult(eProps, (Server) null);
 
         ConfigurationTypeHandler handler = EasyMock.createNiceMock(ConfigurationTypeHandler.class);
-        EasyMock.expect(handler.createServer(sref, bc, sref.getBundle().getBundleContext(), sPropsMod, Runnable.class, svcObject)).andReturn(er).once();
+        EasyMock.expect(handler.createServer(sref, bc, sref.getBundle().getBundleContext(), sProps, Runnable.class, svcObject)).andReturn(er).once();
         EasyMock.replay(handler);
 
         ConfigTypeHandlerFactory handlerFactory = EasyMock.createNiceMock(ConfigTypeHandlerFactory.class);
-        EasyMock.expect(handlerFactory.getHandler(bc, sPropsMod)).andReturn(handler).once(); // Second time shouldn't get there because it should simply copy
+        EasyMock.expect(handlerFactory.getHandler(bc, sProps)).andReturn(handler).once(); // Second time shouldn't get there because it should simply copy
         EasyMock.replay(handlerFactory);
         RemoteServiceAdminCore rsaCore = new RemoteServiceAdminCore(bc, handlerFactory) {};
 
@@ -265,15 +258,20 @@ public class RemoteServiceAdminCoreTest 
         assertEquals("No more exported services", 0, exportedServices.size());
     }
 
-    @Test
-    public void testExportException() throws Exception {
-        BundleContext bc = EasyMock.createNiceMock(BundleContext.class);
-
+    private Bundle createDummyRsaBundle(BundleContext bc) {
         Bundle b = EasyMock.createNiceMock(Bundle.class);
         EasyMock.expect(b.getBundleContext()).andReturn(bc).anyTimes();
         EasyMock.expect(b.getSymbolicName()).andReturn("rsabundle").anyTimes();
         EasyMock.expect(b.getHeaders()).andReturn(new Hashtable<String, String>()).anyTimes();
         EasyMock.replay(b);
+        return b;
+    }
+
+    @Test
+    public void testExportException() throws Exception {
+        BundleContext bc = EasyMock.createNiceMock(BundleContext.class);
+
+        Bundle b = createDummyRsaBundle(bc);
 
         final Map<String, Object> sProps = new HashMap<String, Object>();
         sProps.put("objectClass", new String[] {"java.lang.Runnable"});
@@ -288,24 +286,17 @@ public class RemoteServiceAdminCoreTest 
         EasyMock.expect(bc.getBundle()).andReturn(b).anyTimes();
         EasyMock.replay(bc);
 
-        // The service properties but with Arrays converted into lists (which is what the impl internally uses
-        // to allow these things to be compared).
-        Map<String, Object> sPropsMod = new HashMap<String, Object>();
-        sPropsMod.put("objectClass", Collections.singletonList("java.lang.Runnable"));
-        sPropsMod.put("service.id", 51L);
-        sPropsMod.put("service.exported.interfaces", "*");
-
         HashMap<String, Object> eProps = new HashMap<String, Object>(sProps);
         eProps.put("endpoint.id", "http://something");
         eProps.put("service.imported.configs", new String[] {"org.apache.cxf.ws"});
         ExportResult er = new ExportResult(eProps, new TestException());
 
         ConfigurationTypeHandler handler = EasyMock.createNiceMock(ConfigurationTypeHandler.class);
-        EasyMock.expect(handler.createServer(sref, bc, sref.getBundle().getBundleContext(), sPropsMod, Runnable.class, svcObject)).andReturn(er);
+        EasyMock.expect(handler.createServer(sref, bc, sref.getBundle().getBundleContext(), sProps, Runnable.class, svcObject)).andReturn(er);
         EasyMock.replay(handler);
 
         ConfigTypeHandlerFactory handlerFactory = EasyMock.createNiceMock(ConfigTypeHandlerFactory.class);
-        EasyMock.expect(handlerFactory.getHandler(bc, sPropsMod)).andReturn(handler).anyTimes();
+        EasyMock.expect(handlerFactory.getHandler(bc, sProps)).andReturn(handler).anyTimes();
         EasyMock.replay(handlerFactory);
         RemoteServiceAdminCore rsaCore = new RemoteServiceAdminCore(bc, handlerFactory) {};