logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <rgo...@apache.org>
Subject Re: Asking questions in commit comments.
Date Mon, 26 May 2014 14:00:07 GMT
Thanks Gary, I would have probably missed this.  

The answer to the question is it makes sense to me.  An array should always be fully populated
whereas it is generally OK if a single valued item is null. For example, you might have no
filters but if you do have filters none of them can be null or there is a problem.

Sent from my iPad

> On May 26, 2014, at 6:30 AM, Gary Gregory <garydgregory@gmail.com> wrote:
> 
> Matt,
> 
> In this case, I think you need to ask these questions on the ML with an appropriate subject
instead of buried in a commit message.
> 
> Gary
> 
> ---------- Forwarded message ----------
> From: <mattsicker@apache.org>
> Date: Mon, May 26, 2014 at 2:02 AM
> Subject: svn commit: r1597517 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java
> To: commits@logging.apache.org
> 
> 
> Author: mattsicker
> Date: Mon May 26 06:02:28 2014
> New Revision: 1597517
> 
> URL: http://svn.apache.org/r1597517
> Log:
> Add PluginElement visitor implementation.
> 
>   - This is by far the hardest one to both get right and clean up.
>   I've only been able to clean up a little from the original code
>   this is based on, so please feel free to offer improvements if you
>   can figure this out!
>   - Moved the list of used nodes to directly modify the
>   Node.getChildren() list (outside iteration of course!) instead of
>   relying on a field.
>   - I noticed a bit of inconsistency in how a child node's object is
>   checked for null: when there's an array of elements, null objects
>   are warned about in the log. When there's a single element, a null
>   object is happily returned without any logging. Does this make sense?
> 
> Added:
>     logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java
  (with props)
> 
> Added: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java
> URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java?rev=1597517&view=auto
> ==============================================================================
> --- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java
(added)
> +++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java
Mon May 26 06:02:28 2014
> @@ -0,0 +1,102 @@
> +/*
> + * 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.reflect.Array;
> +import java.util.ArrayList;
> +import java.util.Arrays;
> +import java.util.Collection;
> +import java.util.List;
> +
> +import org.apache.logging.log4j.core.LogEvent;
> +import org.apache.logging.log4j.core.config.Configuration;
> +import org.apache.logging.log4j.core.config.Node;
> +import org.apache.logging.log4j.core.config.plugins.PluginElement;
> +import org.apache.logging.log4j.core.config.plugins.util.PluginType;
> +
> +/**
> + * PluginVisitor implementation for {@link PluginElement}. Supports arrays as well as
singular values.
> + */
> +public class PluginElementVisitor extends AbstractPluginVisitor<PluginElement>
{
> +    public PluginElementVisitor() {
> +        super(PluginElement.class);
> +    }
> +
> +    @Override
> +    public Object visit(final Configuration configuration, final Node node, final LogEvent
event) {
> +        final String name = this.annotation.value();
> +        if (this.conversionType.isArray()) {
> +            setConversionType(this.conversionType.getComponentType());
> +            final List<Object> values = new ArrayList<Object>();
> +            final Collection<Node> used = new ArrayList<Node>();
> +            for (final Node child : node.getChildren()) {
> +                final PluginType<?> childType = child.getType();
> +                if (name.equalsIgnoreCase(childType.getElementName()) ||
> +                    this.conversionType.isAssignableFrom(childType.getPluginClass()))
{
> +                    used.add(child);
> +                    final Object childObject = child.getObject();
> +                    if (childObject == null) {
> +                        LOGGER.error("Null object returned for {} in {}.", child.getName(),
node.getName());
> +                        continue;
> +                    }
> +                    if (childObject.getClass().isArray()) {
> +                        final Object[] o = (Object[]) childObject;
> +                        LOGGER.debug("{}={}", name, Arrays.toString(o));
> +                        return childObject;
> +                    }
> +                    values.add(childObject);
> +                }
> +            }
> +            // note that we need to return an empty array instead of null if the types
are correct
> +            if (!values.isEmpty() && !this.conversionType.isAssignableFrom(values.get(0).getClass()))
{
> +                LOGGER.error("Attempted to assign attribute {} to list of type {} which
is incompatible with {}.",
> +                    name, values.get(0).getClass(), this.conversionType);
> +                return null;
> +            }
> +            node.getChildren().removeAll(used);
> +            LOGGER.debug("{}={}", name, values);
> +            // we need to use reflection here because values.toArray() will cause type
errors at runtime
> +            final Object[] array = (Object[]) Array.newInstance(this.conversionType,
values.size());
> +            for (int i = 0; i < array.length; i++) {
> +                array[i] = values.get(i);
> +            }
> +            return array;
> +        } else {
> +            final Node namedNode = findNamedNode(name, node.getChildren());
> +            if (namedNode == null) {
> +                return null;
> +            }
> +            LOGGER.debug("{}({})", namedNode.getName(), namedNode.toString());
> +            node.getChildren().remove(namedNode);
> +            return namedNode.getObject();
> +        }
> +    }
> +
> +    private Node findNamedNode(final String name, final Iterable<Node> children)
{
> +        for (final Node child : children) {
> +            final PluginType<?> childType = child.getType();
> +            if (name.equalsIgnoreCase(childType.getElementName()) ||
> +                this.conversionType.isAssignableFrom(childType.getPluginClass())) {
> +                // FIXME: check child.getObject() for null?
> +                // doing so would be more consistent with the array version
> +                return child;
> +            }
> +        }
> +        return null;
> +    }
> +}
> 
> Propchange: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginElementVisitor.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
> 
> 
> 
> 
> 
> -- 
> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
> Java Persistence with Hibernate, Second Edition
> JUnit in Action, Second Edition
> Spring Batch in Action
> Blog: http://garygregory.wordpress.com 
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

Mime
View raw message