Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/492#discussion_r93619410
  
    --- Diff: 
core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingClassRenamesProvider.java
 ---
    @@ -20,65 +20,217 @@
     
     import java.io.IOException;
     import java.io.InputStream;
    +import java.util.Dictionary;
     import java.util.Enumeration;
    +import java.util.List;
     import java.util.Map;
     import java.util.Properties;
     
    +import org.apache.brooklyn.util.collections.MutableMap;
     import org.apache.brooklyn.util.core.ResourceUtils;
     import org.apache.brooklyn.util.exceptions.Exceptions;
     import org.apache.brooklyn.util.javalang.Reflections;
     import org.apache.brooklyn.util.stream.Streams;
    +import org.osgi.framework.Constants;
    +import org.osgi.framework.InvalidSyntaxException;
    +import org.osgi.service.cm.Configuration;
    +import org.osgi.service.cm.ConfigurationAdmin;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
     
     import com.google.common.annotations.Beta;
    +import com.google.common.collect.ImmutableList;
     import com.google.common.collect.ImmutableMap;
    +import com.google.common.collect.Lists;
     import com.google.common.collect.Maps;
     
     @Beta
     public class DeserializingClassRenamesProvider {
     
    +    /*
    +     * This provider keeps a cache of the class-renames, which is lazily 
populated (see {@link #cache}. 
    +     * Calling {@link #reset()} will set this cache to null, causing it to 
be reloaded next time 
    +     * it is requested.
    +     * 
    +     * Loading the cache involves iterating over the {@link #loaders}, 
returning the union of 
    +     * the results from {@link Loader#load()}.
    +     * 
    +     * Initially, the only loader is the basic {@link 
ClasspathConfigLoader}.
    +     * 
    +     * However, when running in karaf the {@link OsgiConfigLoader} will be 
instantiated and added.
    +     * See karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml
    +     */
    +    
    +    private static final Logger LOG = 
LoggerFactory.getLogger(DeserializingClassRenamesProvider.class);
    +    private static final List<String> EXCLUDED_KEYS = 
ImmutableList.of("service.pid", "felix.fileinstall.filename");
    +
         public static final String DESERIALIZING_CLASS_RENAMES_PROPERTIES_PATH 
= 
"classpath://org/apache/brooklyn/core/mgmt/persist/deserializingClassRenames.properties";
    +    public static final String 
KARAF_DESERIALIZING_CLASS_RENAMES_PROPERTIES = 
"org.apache.brooklyn.classrename";
    +
    +    private static final List<Loader> loaders = 
Lists.newCopyOnWriteArrayList();
    +    static {
    +        loaders.add(new ClasspathConfigLoader());
    +    }
         
         private static volatile Map<String, String> cache;
    -    
    +
         @Beta
         public static Map<String, String> loadDeserializingClassRenames() {
    -        // Double-checked locking - got to use volatile or some such!
    -        if (cache == null) {
    -            synchronized (DeserializingClassRenamesProvider.class) {
    -                if (cache == null) {
    -                    cache = loadDeserializingClassRenamesCache();
    +        synchronized (DeserializingClassRenamesProvider.class) {
    +            if (cache == null) {
    +                MutableMap.Builder<String, String> builder = 
MutableMap.<String, String>builder();
    +                for (Loader loader : loaders) {
    +                    builder.putAll(loader.load());
                     }
    +                cache = builder.build();
    +                LOG.info("Class-renames cache loaded, size {}", 
cache.size());
                 }
    +            return cache;
             }
    -        return cache;
    +    }
    +
    +    /**
    +     * Handles inner classes, where the outer class has been renamed. For 
example:
    +     * 
    +     * {@code findMappedName("com.example.MyFoo$MySub")} will return 
{@code com.example.renamed.MyFoo$MySub}, if
    +     * the renamed contains {@code com.example.MyFoo: 
com.example.renamed.MyFoo}.
    +     */
    +    @Beta
    +    public static String findMappedName(String name) {
    +        return 
Reflections.findMappedNameAndLog(DeserializingClassRenamesProvider.loadDeserializingClassRenames(),
 name);
    +    }
    +
    +    public static void reset() {
    +        synchronized (DeserializingClassRenamesProvider.class) {
    +            cache = null;
    +        }
    +    }
    +
    +    public interface Loader {
    +        public Map<String, String> load();
         }
         
    -    private static Map<String, String> 
loadDeserializingClassRenamesCache() {
    -        InputStream resource = new 
ResourceUtils(DeserializingClassRenamesProvider.class).getResourceFromUrl(DESERIALIZING_CLASS_RENAMES_PROPERTIES_PATH);
    -        if (resource != null) {
    +    /**
    +     * Loads the class-renames from the OSGi configuration file: {@code 
org.apache.brooklyn.classrename.cfg}.
    +     * 
    +     * Only public for OSGi instantiation - treat as an internal class, 
which may change in 
    +     * future releases.
    +     * 
    +     * See 
http://stackoverflow.com/questions/18844987/creating-a-blueprint-bean-from-an-inner-class:
    +     * we unfortunately need to include {@code 
!org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider}
    +     * in the Import-Package, as the mvn plugin gets confused due to the 
use of this inner class
    +     * within the blueprint.xml.
    +     * 
    +     * @see {@link 
DeserializingClassRenamesProvider#KARAF_DESERIALIZING_CLASS_RENAMES_PROPERTIES}
    +     */
    +    public static class OsgiConfigLoader implements Loader {
    +        private ConfigurationAdmin configAdmin;
    +
    +        public OsgiConfigLoader() {
    +            LOG.trace("OsgiConfigLoader instance created");
    +        }
    +
    +        // For injection as OSGi bean
    +        public void setConfigAdmin(ConfigurationAdmin configAdmin) {
    +            this.configAdmin = configAdmin;
    +        }
    +
    +        // Called by OSGi
    +        public void init() {
    +            
LOG.trace("DeserializingClassRenamesProvider.OsgiConfigLoader.init: registering 
loader");
    +            DeserializingClassRenamesProvider.loaders.add(this);
    +            DeserializingClassRenamesProvider.reset();
    +        }
    +
    +        // Called by OSGi
    +        public void destroy() {
    +            
LOG.trace("DeserializingClassRenamesProvider.OsgiConfigLoader.destroy: 
unregistering loader");
    +            boolean removed = 
DeserializingClassRenamesProvider.loaders.remove(this);
    +            if (removed) {
    +                DeserializingClassRenamesProvider.reset();
    +            }
    +        }
    +
    +        // Called by OSGi when configuration changes
    +        public void updateProperties(Map properties) {
    +            
LOG.debug("DeserializingClassRenamesProvider.OsgiConfigLoader.updateProperties: 
clearing cache, so class-renames will be reloaded");
    +            DeserializingClassRenamesProvider.reset();
    +        }
    +
    +        @Override
    +        public Map<String, String> load() {
    +            if (configAdmin == null) {
    +                LOG.warn("No OSGi configuration-admin available - cannot 
load {}.cfg", KARAF_DESERIALIZING_CLASS_RENAMES_PROPERTIES);
    +                return ImmutableMap.of();
    +            }
    +            
    +            String filter = '(' + Constants.SERVICE_PID + '=' + 
KARAF_DESERIALIZING_CLASS_RENAMES_PROPERTIES + ')';
    --- End diff --
    
    Add a TODO here, you could avoid using ConfigurationAdmin in the code and 
doing all that filtering/ listConfigurations yourself. It should be possible to 
get cm to do that for you with a managed bean;  but  it’s not a big deal, it 
just means you have written a little more code  than you needed to.
    
    The way the configuration admin service is typically intended to be used is 
that you let it inject properties into the beans that you declare - i.e. your 
code doesn't need to get hold of the ConfigurationService class at all.  See 
https://www.eclipse.org/gemini/blueprint/documentation/reference/2.0.0.RELEASE/html/compendium.html
 section 11.1.2.1 for background.
    
    I was thinking along the lines of changing the 
DeserializingClassRenamesProvider to be a regular bean that could be injected 
as a service, but the pattern of callers would make that a quite extensive 
change, affecting a lot of classes.  But it shouldn't be hard to make it a 
singleton bean, and have it be initialised by the launchers. Then in Karaf 
(OsgiLauncher) you would have an "DeserializingClassRenamesProviderInitializer" 
managed bean in the blueprint.xml,
    
    ```xml
    <bean id="deserializingRenamesInit" 
class="org.apache.brooklyn.launcher.osgi.DeserializingClassRenamesProviderInitializer">
       <osgix:managed-properties 
persistent-id="org.apache.brooklyn.core.deserializing.renames" 
update-method="updateCallback"/>
    </bean>
    ```
    
    and then the DeserializingClassRenamesProviderInitializer would have a 
method to initialize the provider, along the following lines and ignoring all 
niceties of synchronization, exceptions etc.
    
    ```java
    public class DeserializingClassRenamesProviderInitializer {
    
        public void updateCallback(Map properties) {
    
            DeserializingClassRenamesProvider.initializeSingleton(properties);
        }
    }
    ```
    
    and changing the provider class to
    
    ```java
    public class DeserializingClassRenamesProvider {
    
        private volatile Map<String, String> cache;
    
        public static DeserializingClassRenamesProvider INSTANCE;
    
        private DeserializingClassRenamesProvider(Map properties) {
            // reset cache from properties
        }
    
        public static void initializeSingleton(Map properties) {
            INSTANCE = new DeserializingClassRenamesProvider(properties);
        }
    
        // not static!
        public Map<String, String> loadDeserializingClassRenames() {
           // return the cache
        }
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to