I actually had a better idea how to handle this on the train this morning. Instead of having a registry for the plugin visitors, I thought it might be easier to just use a meta-annotation that contains the FQCN of the visitor method. That way we can use the ClassLoader on the annotation itself to load the visitor class and therefore make it instantly OSGi-friendly.
I'm just running the unit tests right now before I commit anything of course, but I think it's a bit more elegant. With that in mind, I think renaming it back to PluginVisitors now makes sense as all it would contain is a utility method regarding PluginVisitor classes. On 26 May 2014 23:16, Gary Gregory <[email protected]> wrote: > Clearer is better here. +1 to "registry". > > Gary > > > -------- Original message -------- > From: Matt Sicker > Date:05/26/2014 19:49 (GMT-05:00) > To: Log4J Developers List > Subject: Re: svn commit: r1597509 - > /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java > > > I think you're right. > > > On 26 May 2014 18:45, Remko Popma <[email protected]> wrote: > >> Is PluginVisitorRegistry not a better name than PluginVisitors? >> The convention to append "s" after another class name has many meanings, >> like providing static util methods. >> I think calling it a ..Registry is clearer. >> >> Sent from my iPhone >> >> > On 2014/05/26, at 12:59, [email protected] wrote: >> > >> > Author: mattsicker >> > Date: Mon May 26 03:59:52 2014 >> > New Revision: 1597509 >> > >> > URL: http://svn.apache.org/r1597509 >> > Log: >> > Add PluginVisitors registry class. >> > >> > - Registers the PluginAttribute visitor by default. >> > - Will be adding other visitor implementations over time. >> > >> > Added: >> > >> >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java >> (with props) >> > >> > Added: >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java >> > URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java?rev=1597509&view=auto >> > >> ============================================================================== >> > --- >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java >> (added) >> > +++ >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java >> Mon May 26 03:59:52 2014 >> > @@ -0,0 +1,81 @@ >> > +/* >> > + * 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.logging.log4j.core.config.plugins.visitors; >> > + >> > +import java.lang.annotation.Annotation; >> > +import java.util.Map; >> > +import java.util.concurrent.ConcurrentHashMap; >> > + >> > +import org.apache.logging.log4j.Logger; >> > +import org.apache.logging.log4j.core.config.plugins.PluginAttribute; >> > +import org.apache.logging.log4j.status.StatusLogger; >> > + >> > +/** >> > + * Registry for associating Plugin annotations with PluginVisitor >> implementations. >> > + */ >> > +public final class PluginVisitors { >> > + >> > + private static final Logger LOGGER = StatusLogger.getLogger(); >> > + >> > + // map of annotation classes to their corresponding PluginVisitor >> classes >> > + // generics are fun! >> > + private static final Map<Class<? extends Annotation>, Class<? >> extends PluginVisitor<? extends Annotation>>> REGISTRY; >> > + >> > + static { >> > + // register the default PluginVisitor classes >> > + REGISTRY = new ConcurrentHashMap<Class<? extends Annotation>, >> Class<? extends PluginVisitor<? extends Annotation>>>(); >> > + registerVisitor(PluginAttribute.class, >> PluginAttributeVisitor.class); >> > + } >> > + >> > + private PluginVisitors() { >> > + } >> > + >> > + /** >> > + * Registers a PluginVisitor class associated to a specific >> annotation. >> > + * >> > + * @param annotation the Plugin annotation to associate with. >> > + * @param helper the PluginVisitor class to use for the >> annotation. >> > + * @param <A> the Plugin annotation type. >> > + */ >> > + public static <A extends Annotation> void registerVisitor(final >> Class<A> annotation, >> > + final >> Class<? extends PluginVisitor<A>> helper) { >> > + REGISTRY.put(annotation, helper); >> > + } >> > + >> > + /** >> > + * Creates a PluginVisitor instance for the given annotation >> class. This instance must be further populated with >> > + * data to be useful. Such data is passed through both the setters >> and the visit method. >> > + * >> > + * @param annotation the Plugin annotation class to find a >> PluginVisitor for. >> > + * @param <A> the Plugin annotation type. >> > + * @return a PluginVisitor instance if one could be created, or >> {@code null} otherwise. >> > + */ >> > + @SuppressWarnings("unchecked") // we're keeping track of types, >> thanks >> > + public static <A extends Annotation> PluginVisitor<A> >> findVisitor(final Class<A> annotation) { >> > + try { >> > + final Class<PluginVisitor<A>> clazz = >> (Class<PluginVisitor<A>>) REGISTRY.get(annotation); >> > + if (clazz == null) { >> > + return null; >> > + } >> > + return clazz.newInstance(); >> > + } catch (final Exception e) { >> > + LOGGER.debug("No PluginVisitor found for annotation: {}.", >> annotation); >> > + return null; >> > + } >> > + } >> > +} >> > >> > Propchange: >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java >> > >> ------------------------------------------------------------------------------ >> > svn:eol-style = native >> > >> > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >> > > > -- > Matt Sicker <[email protected]> > -- Matt Sicker <[email protected]>
