logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: svn commit: r1597790 - in /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins: ./ util/ visitors/
Date Tue, 27 May 2014 15:49:25 GMT
You may want to document that you use String FQCNs on purpose even though
annotations let you use Classes, to make it OSGi-friendly. Otherwise the
next person may refactor this...


On Tue, May 27, 2014 at 10:57 PM, <mattsicker@apache.org> wrote:

> Author: mattsicker
> Date: Tue May 27 13:57:31 2014
> New Revision: 1597790
>
> URL: http://svn.apache.org/r1597790
> Log:
> Switch from a registry-based strategy to a meta-annotation-based strategy
> for associating visitors to annotations.
>
>   - More easily extensible as there is no registry or special file to
> maintain.
>   - Still maintains OSGi compatibility with correct ClassLoader usage.
>   - Updated PluginVisitor(Builder|s) accordingly to use new algorithm.
>
> Added:
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginVisitorStrategy.java
>   (with props)
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java
>       - copied, changed from r1597667,
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitorRegistry.java
> Removed:
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitorRegistry.java
> Modified:
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginAttribute.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginConfiguration.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginElement.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginNode.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/SensitivePluginAttribute.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginBuilder.java
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/package-info.java
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginAttribute.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginAttribute.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginAttribute.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginAttribute.java
> Tue May 27 13:57:31 2014
> @@ -32,6 +32,7 @@ import java.lang.annotation.Target;
>  @Documented
>  @Retention(RetentionPolicy.RUNTIME)
>  @Target({ElementType.PARAMETER, ElementType.FIELD})
>
> +@PluginVisitorStrategy("org.apache.logging.log4j.core.config.plugins.visitors.PluginAttributeVisitor")
>  public @interface PluginAttribute {
>
>      // TODO: could we allow a blank value and infer the attribute name
> through reflection?
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginConfiguration.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginConfiguration.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginConfiguration.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginConfiguration.java
> Tue May 27 13:57:31 2014
> @@ -29,5 +29,6 @@ import java.lang.annotation.Target;
>  @Documented
>  @Retention(RetentionPolicy.RUNTIME)
>  @Target({ElementType.PARAMETER, ElementType.FIELD})
>
> +@PluginVisitorStrategy("org.apache.logging.log4j.core.config.plugins.visitors.PluginConfigurationVisitor")
>  public @interface PluginConfiguration {
>  }
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginElement.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginElement.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginElement.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginElement.java
> Tue May 27 13:57:31 2014
> @@ -28,6 +28,7 @@ import java.lang.annotation.Target;
>  @Documented
>  @Retention(RetentionPolicy.RUNTIME)
>  @Target({ElementType.PARAMETER, ElementType.FIELD})
>
> +@PluginVisitorStrategy("org.apache.logging.log4j.core.config.plugins.visitors.PluginElementVisitor")
>  public @interface PluginElement {
>
>      /**
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginNode.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginNode.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginNode.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginNode.java
> Tue May 27 13:57:31 2014
> @@ -28,5 +28,6 @@ import java.lang.annotation.Target;
>  @Documented
>  @Retention(RetentionPolicy.RUNTIME)
>  @Target({ElementType.PARAMETER, ElementType.FIELD})
>
> +@PluginVisitorStrategy("org.apache.logging.log4j.core.config.plugins.visitors.PluginNodeVisitor")
>  public @interface PluginNode {
>  }
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
> Tue May 27 13:57:31 2014
> @@ -31,6 +31,7 @@ import java.lang.annotation.Target;
>  @Documented
>  @Retention(RetentionPolicy.RUNTIME)
>  @Target(ElementType.PARAMETER)
>
> +@PluginVisitorStrategy("org.apache.logging.log4j.core.config.plugins.visitors.PluginValueVisitor")
>  public @interface PluginValue {
>
>      String value();
>
> Added:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginVisitorStrategy.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginVisitorStrategy.java?rev=1597790&view=auto
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginVisitorStrategy.java
> (added)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginVisitorStrategy.java
> Tue May 27 13:57:31 2014
> @@ -0,0 +1,40 @@
> +/*
> + * 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;
> +
> +import java.lang.annotation.Documented;
> +import java.lang.annotation.ElementType;
> +import java.lang.annotation.Retention;
> +import java.lang.annotation.RetentionPolicy;
> +import java.lang.annotation.Target;
> +
> +/**
> + * Meta-annotation to denote the class name to use that implements
> + * {@link
> org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitor} for
> the annotated annotation.
> + */
> +@Documented
> +@Retention(RetentionPolicy.RUNTIME)
> +@Target(ElementType.ANNOTATION_TYPE)
> +public @interface PluginVisitorStrategy {
> +
> +    /**
> +     * The class name to use that implements {@link
> org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitor}
> +     * for the given annotation.
> +     */
> +    String value();
> +}
>
> Propchange:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginVisitorStrategy.java
>
> ------------------------------------------------------------------------------
>     svn:eol-style = native
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/SensitivePluginAttribute.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/SensitivePluginAttribute.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/SensitivePluginAttribute.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/SensitivePluginAttribute.java
> Tue May 27 13:57:31 2014
> @@ -34,6 +34,7 @@ import java.lang.annotation.Target;
>  @Documented
>  @Retention(RetentionPolicy.RUNTIME)
>  @Target(ElementType.PARAMETER)
>
> +@PluginVisitorStrategy("org.apache.logging.log4j.core.config.plugins.visitors.SensitivePluginAttributeVisitor")
>  public @interface SensitivePluginAttribute {
>      String value();
>
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginBuilder.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginBuilder.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginBuilder.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginBuilder.java
> Tue May 27 13:57:31 2014
> @@ -34,7 +34,7 @@ import org.apache.logging.log4j.core.con
>  import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
>  import org.apache.logging.log4j.core.config.plugins.PluginFactory;
>  import
> org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitor;
> -import
> org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitorRegistry;
> +import
> org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitors;
>  import org.apache.logging.log4j.core.util.Assert;
>  import org.apache.logging.log4j.core.util.Builder;
>  import org.apache.logging.log4j.status.StatusLogger;
> @@ -164,7 +164,7 @@ public class PluginBuilder<T> implements
>                      continue; // already processed
>                  }
>                  final PluginVisitor<? extends Annotation> visitor =
> -                    PluginVisitorRegistry.findVisitor(a.annotationType());
> +                    PluginVisitors.findVisitor(a.annotationType());
>                  if (visitor != null) {
>                      final Object value = visitor.setAliases(aliases)
>                          .setAnnotation(a)
> @@ -200,7 +200,7 @@ public class PluginBuilder<T> implements
>                  if (a instanceof PluginAliases) {
>                      continue; // already processed
>                  }
> -                final PluginVisitor<? extends Annotation> visitor =
> PluginVisitorRegistry.findVisitor(
> +                final PluginVisitor<? extends Annotation> visitor =
> PluginVisitors.findVisitor(
>                      a.annotationType());
>                  if (visitor != null) {
>                      args[i] = visitor.setAliases(aliases)
>
> Copied:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java
> (from r1597667,
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitorRegistry.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?p2=logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java&p1=logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitorRegistry.java&r1=1597667&r2=1597790&rev=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitorRegistry.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginVisitors.java
> Tue May 27 13:57:31 2014
> @@ -18,58 +18,24 @@
>  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.core.config.plugins.PluginConfiguration;
> -import org.apache.logging.log4j.core.config.plugins.PluginElement;
> -import org.apache.logging.log4j.core.config.plugins.PluginNode;
> -import org.apache.logging.log4j.core.config.plugins.PluginValue;
> -import
> org.apache.logging.log4j.core.config.plugins.SensitivePluginAttribute;
> +import org.apache.logging.log4j.core.config.plugins.PluginVisitorStrategy;
>  import org.apache.logging.log4j.status.StatusLogger;
>
>  /**
> - * Registry for associating Plugin annotations with PluginVisitor
> implementations.
> + * Utility class to locate an appropriate PluginVisitor implementation
> for an annotation.
>   */
> -public final class PluginVisitorRegistry {
> +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
> -        // TODO: this could probably be combined with the usual plugin
> architecture instead
> -        REGISTRY = new ConcurrentHashMap<Class<? extends Annotation>,
> Class<? extends PluginVisitor<? extends Annotation>>>();
> -        registerVisitor(PluginAttribute.class,
> PluginAttributeVisitor.class);
> -        registerVisitor(SensitivePluginAttribute.class,
> SensitivePluginAttributeVisitor.class);
> -        registerVisitor(PluginConfiguration.class,
> PluginConfigurationVisitor.class);
> -        registerVisitor(PluginNode.class, PluginNodeVisitor.class);
> -        registerVisitor(PluginValue.class, PluginValueVisitor.class);
> -        registerVisitor(PluginElement.class, PluginElementVisitor.class);
> -    }
> -
> -    private PluginVisitorRegistry() {
> -    }
> -
> -    /**
> -     * 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);
> +    private PluginVisitors() {
>      }
>
>      /**
> -     * Creates a PluginVisitor instance for the given annotation class.
> This instance must be further populated with
> +     * Creates a PluginVisitor instance for the given annotation class
> using metadata provided by the annotation's
> +     * {@link PluginVisitorStrategy} annotation. 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.
> @@ -78,14 +44,21 @@ public final class PluginVisitorRegistry
>       */
>      @SuppressWarnings("unchecked") // we're keeping track of types, thanks
>      public static <A extends Annotation> PluginVisitor<A>
> findVisitor(final Class<A> annotation) {
> +        final PluginVisitorStrategy strategy =
> annotation.getAnnotation(PluginVisitorStrategy.class);
> +        if (strategy == null) {
> +            LOGGER.debug("No PluginVisitorStrategy found on annotation
> [{}]. Ignoring.", annotation);
> +            return null;
> +        }
> +        final String visitorClassName = strategy.value();
>          try {
> -            final Class<PluginVisitor<A>> clazz =
> (Class<PluginVisitor<A>>) REGISTRY.get(annotation);
> -            if (clazz == null) {
> -                return null;
> -            }
> -            return clazz.newInstance();
> +            // if a PluginVisitor is in a different JAR than log4j-core,
> it can be safely assumed that the
> +            // corresponding annotation is in the same JAR as the
> PluginVisitor implementation. thus, we use that
> +            // ClassLoader instead of any default one
> +            final Class<? extends PluginVisitor<A>> visitorClass =
> +                (Class<? extends PluginVisitor<A>>)
> annotation.getClassLoader().loadClass(visitorClassName);
> +            return visitorClass.newInstance();
>          } catch (final Exception e) {
> -            LOGGER.debug("No PluginVisitor found for annotation: {}.",
> annotation);
> +            LOGGER.error("Error loading PluginVisitor [{}] for annotation
> [{}].", visitorClassName, annotation, e);
>              return null;
>          }
>      }
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/package-info.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/package-info.java?rev=1597790&r1=1597789&r2=1597790&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/package-info.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/package-info.java
> Tue May 27 13:57:31 2014
> @@ -17,5 +17,8 @@
>
>  /**
>   * Visitor classes for extracting values from a Configuration or Node
> corresponding to a plugin annotation.
> + * Visitor implementations must implement {@link
> org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitor},
> + * and the corresponding annotation must be annotated with
> + * {@link
> org.apache.logging.log4j.core.config.plugins.PluginVisitorStrategy}.
>   */
>  package org.apache.logging.log4j.core.config.plugins.visitors;
> \ No newline at end of file
>
>
>

Mime
View raw message