Instead of Map::isEmpty, we could do Collection::isEmpty. Also is it a good idea to call Iterator::hasNext? In some implementations calling this method moves the cursor one position forward. Do we actually need to check that an Iterator is empty anywhere?
Sent from my iPhone > On Jan 7, 2017, at 12:51, mattsic...@apache.org wrote: > > Repository: logging-log4j2 > Updated Branches: > refs/heads/master b0daba63f -> 61b3ab301 > > > Extract isEmpty() logic from RequiredValidator > > > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/61b3ab30 > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/61b3ab30 > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/61b3ab30 > > Branch: refs/heads/master > Commit: 61b3ab301a4d54c28e736ca3a37f984475969377 > Parents: b0daba6 > Author: Matt Sicker <matt.sic...@spr.com> > Authored: Fri Jan 6 21:50:49 2017 -0600 > Committer: Matt Sicker <matt.sic...@spr.com> > Committed: Fri Jan 6 21:50:49 2017 -0600 > > ---------------------------------------------------------------------- > .../validators/RequiredValidator.java | 24 +------ > .../apache/logging/log4j/core/util/Assert.java | 46 +++++++++++++ > .../logging/log4j/core/util/AssertTest.java | 68 ++++++++++++++++++++ > 3 files changed, 116 insertions(+), 22 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61b3ab30/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/validation/validators/RequiredValidator.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/validation/validators/RequiredValidator.java > > b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/validation/validators/RequiredValidator.java > index 5e6cec0..98c0a71 100644 > --- > a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/validation/validators/RequiredValidator.java > +++ > b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/validation/validators/RequiredValidator.java > @@ -22,6 +22,7 @@ import java.util.Map; > import org.apache.logging.log4j.Logger; > import > org.apache.logging.log4j.core.config.plugins.validation.ConstraintValidator; > import > org.apache.logging.log4j.core.config.plugins.validation.constraints.Required; > +import org.apache.logging.log4j.core.util.Assert; > import org.apache.logging.log4j.status.StatusLogger; > > /** > @@ -49,28 +50,7 @@ public class RequiredValidator implements > ConstraintValidator<Required> { > > @Override > public boolean isValid(final String name, final Object value) { > - if (value == null) { > - return err(name); > - } > - if (value instanceof CharSequence) { > - final CharSequence sequence = (CharSequence) value; > - return sequence.length() != 0 || err(name); > - } > - final Class<?> clazz = value.getClass(); > - if (clazz.isArray()) { > - final Object[] array = (Object[]) value; > - return array.length != 0 || err(name); > - } > - if (Collection.class.isAssignableFrom(clazz)) { > - final Collection<?> collection = (Collection<?>) value; > - return collection.size() != 0 || err(name); > - } > - if (Map.class.isAssignableFrom(clazz)) { > - final Map<?, ?> map = (Map<?, ?>) value; > - return map.size() != 0 || err(name); > - } > - // LOGGER.debug("Encountered type [{}] which can only be checked for > null.", clazz.getName()); > - return true; > + return Assert.isNonEmpty(value) || err(name); > } > > private boolean err(final String name) { > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61b3ab30/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > index e37090f..d46c06a 100644 > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > @@ -16,6 +16,8 @@ > */ > package org.apache.logging.log4j.core.util; > > +import java.util.Map; > + > /** > * Utility class providing common validation logic. > */ > @@ -23,6 +25,50 @@ public final class Assert { > private Assert() { > } > > + /** > + * Checks if an object has empty semantics. The following scenarios are > considered empty: > + * <ul> > + * <li>{@code null}</li> > + * <li>empty {@link CharSequence}</li> > + * <li>empty array</li> > + * <li>empty {@link Iterable}</li> > + * <li>empty {@link Map}</li> > + * </ul> > + * > + * @param o value to check for emptiness > + * @return true if the value is empty, false otherwise > + * @since 2.8 > + */ > + public static boolean isEmpty(final Object o) { > + if (o == null) { > + return true; > + } > + if (o instanceof CharSequence) { > + return ((CharSequence) o).length() == 0; > + } > + if (o.getClass().isArray()) { > + return ((Object[]) o).length == 0; > + } > + if (o instanceof Iterable) { > + return !((Iterable<?>) o).iterator().hasNext(); > + } > + if (o instanceof Map) { > + return ((Map<?, ?>) o).isEmpty(); > + } > + return false; > + } > + > + /** > + * Opposite of {@link #isEmpty(Object)}. > + * > + * @param o value to check for non-emptiness > + * @return true if the value is non-empty, false otherwise > + * @since 2.8 > + */ > + public static boolean isNonEmpty(final Object o) { > + return !isEmpty(o); > + } > + > public static int valueIsAtLeast(final int value, final int minValue) { > if (value < minValue) { > throw new IllegalArgumentException("Value should be at least " + > minValue + " but was " + value); > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61b3ab30/log4j-core/src/test/java/org/apache/logging/log4j/core/util/AssertTest.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/AssertTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/AssertTest.java > new file mode 100644 > index 0000000..242c41e > --- /dev/null > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/AssertTest.java > @@ -0,0 +1,68 @@ > +/* > + * 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.util; > + > +import java.util.ArrayList; > +import java.util.Collections; > +import java.util.HashMap; > + > +import org.junit.Test; > +import org.junit.runner.RunWith; > +import org.junit.runners.Parameterized; > + > +import static org.junit.Assert.*; > + > +/** > + * > + */ > +@RunWith(Parameterized.class) > +public class AssertTest { > + > + private final Object value; > + private final boolean isEmpty; > + > + @Parameterized.Parameters > + public static Object[][] data() { > + return new Object[][]{ > + // value, isEmpty > + {null, true}, > + {"", true}, > + {new Object[0], true}, > + {new ArrayList<>(), true}, > + {new HashMap<>(), true}, > + {0, false}, > + {1, false}, > + {false, false}, > + {true, false}, > + {new Object[]{null}, false}, > + {Collections.singletonList(null), false}, > + {Collections.singletonMap("", null), false}, > + {"null", false} > + }; > + } > + > + public AssertTest(final Object value, final boolean isEmpty) { > + this.value = value; > + this.isEmpty = isEmpty; > + } > + > + @Test > + public void isEmpty() throws Exception { > + assertEquals(isEmpty, Assert.isEmpty(value)); > + } > + > +} > \ No newline at end of file > --------------------------------------------------------------------- To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org
