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 <[email protected]> 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: <[email protected]> > 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: [email protected] > > > 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: [email protected] | [email protected] > 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
