garydgregory commented on code in PR #412:
URL:
https://github.com/apache/commons-configuration/pull/412#discussion_r1590338254
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -429,6 +443,23 @@ public final boolean containsKey(final String key) {
*/
protected abstract boolean containsKeyInternal(String key);
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the {@link #containsKeyInternal
containsKey} method.
+ * <p>
+ * The implementation of this method will be different depending on the
type of Configuration used.
+ *
Review Comment:
Close HTML paragraph tags.
##########
src/main/java/org/apache/commons/configuration2/DynamicCombinedConfiguration.java:
##########
@@ -358,6 +358,16 @@ protected boolean containsKeyInternal(final String key) {
return this.getCurrentConfig().containsKey(key);
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/test/java/org/apache/commons/configuration2/web/TestServletFilterConfiguration.java:
##########
@@ -91,4 +92,10 @@ public void testAddPropertyDirect() {
public void testClearProperty() {
assertThrows(UnsupportedOperationException.class,
super::testClearProperty);
}
+
+ @Test
+ public void
givenNullValue_testContainsValue_shouldThrowNullPointerException() {
+ final Configuration config = getConfiguration();
+ assertThrows(NullPointerException.class, () ->
config.containsValue(null), "should throw NullPointerException");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -1517,6 +1548,23 @@ protected int sizeInternal() {
return size;
}
+ /**
+ * Checks if the specified value exists in the properties structure mapped
by the provided keys.
+ *
+ * @param keys an Iterator of String keys to search for the value
+ * @param value the String value to search for in the properties
+ * @return true if the value is found in the properties, false otherwise
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/ImmutableConfiguration.java:
##########
@@ -62,6 +62,24 @@ public interface ImmutableConfiguration {
*/
boolean containsKey(String key);
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the {@link #containsKey
containsKey} method.
+ *
+ * @param value value whose presence in this configuration is to be tested
+ * @return {@code true} if this configuration maps one or more keys to the
specified value, false otherwise.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/BaseConfiguration.java:
##########
@@ -136,6 +136,16 @@ protected boolean containsKeyInternal(final String key) {
return store.containsKey(key);
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/AbstractHierarchicalConfiguration.java:
##########
@@ -455,6 +455,16 @@ protected boolean containsKeyInternal(final String key) {
return getPropertyInternal(key) != null;
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/DatabaseConfiguration.java:
##########
@@ -478,6 +478,25 @@ protected Boolean performOperation() throws SQLException {
return result != null && result.booleanValue();
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/DataConfiguration.java:
##########
@@ -213,6 +213,16 @@ protected boolean containsKeyInternal(final String key) {
return configuration.containsKey(key);
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -429,6 +443,23 @@ public final boolean containsKey(final String key) {
*/
protected abstract boolean containsKeyInternal(String key);
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the {@link #containsKeyInternal
containsKey} method.
+ * <p>
+ * The implementation of this method will be different depending on the
type of Configuration used.
+ *
+ * <p>Note that this method is identical in functionality to
+ * {@link #containsValue containsValue}, (which is part of the {@link
ImmutableConfiguration} interface).
+ *
+ * @param value a value to search for
+ * @return {@code true} if and only if some key maps to the {@code value}
argument in this hashtable as determined
+ * by the {@code equals} method; {@code false} otherwise.
+ * @throws NullPointerException if the value is {@code null}
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/PatternSubtreeConfigurationWrapper.java:
##########
@@ -130,6 +130,16 @@ protected boolean containsKeyInternal(final String key) {
return config.containsKey(makePath(key));
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/CompositeConfiguration.java:
##########
@@ -298,6 +298,16 @@ protected boolean containsKeyInternal(final String key) {
return configList.stream().anyMatch(config -> config.containsKey(key));
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/JNDIConfiguration.java:
##########
@@ -147,6 +147,16 @@ protected boolean containsKeyInternal(String key) {
}
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first match
+ * but may be more expensive than the containsKey method.
+ * @since 2.0
+ */
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -419,6 +419,20 @@ public final boolean containsKey(final String key) {
}
}
+ /**
+ * {@inheritDoc} This implementation handles synchronization and delegates
to {@code containsKeyInternal()}.
+ * @since 2.0
Review Comment:
This `@since` is is not correct.
##########
src/test/java/org/apache/commons/configuration2/TestBaseConfiguration.java:
##########
@@ -717,4 +717,9 @@ public void testSubset() {
public void testThrowExceptionOnMissing() {
assertTrue(config.isThrowExceptionOnMissing());
}
+
+ @Test
+ public void givenNullValue_testContainsValue_shouldReturnFalse() {
+ assertFalse(config.containsValue(null), "should return false");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/TestAbstractConfiguration.java:
##########
@@ -78,6 +78,35 @@ public void testAddPropertyDirect() {
assertEquals(expected, list);
}
+ @Test
+ public void testContainsValue(){
+ final Configuration config = getConfiguration();
+ assertTrue(config.containsValue("value1"), "should return true for
class " + this.getClass());
+ assertFalse(config.containsValue("value99999"), "should return false
for class " + this.getClass());
+ }
+
+ @Test
+ public void testContains(){
+ final AbstractConfiguration config = getConfiguration();
+ assertTrue(config.contains(config.getKeys(), "value1"), "should return
true for class " + this.getClass());
+ assertFalse(config.contains(config.getKeys(),"value99999"), "should
return false for class " + this.getClass());
+ }
+
+ @Test
+ public void
givenNullIterator_testContains_shouldThrowNullPointerException(){
+ final AbstractConfiguration config = getConfiguration();
+
+ assertThrows(NullPointerException.class, () ->
config.contains(null,"value1"));
Review Comment:
Missing space after `,`.
##########
src/main/java/org/apache/commons/configuration2/web/BaseWebConfiguration.java:
##########
@@ -68,6 +68,16 @@ protected boolean containsKeyInternal(final String key) {
return getPropertyInternal(key) != null;
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first match
+ * but may be more expensive than the containsKey method
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/main/java/org/apache/commons/configuration2/MapConfiguration.java:
##########
@@ -172,6 +172,16 @@ protected boolean containsKeyInternal(final String key) {
return map.containsKey(key);
}
+ /**
+ * Tests whether this configuration contains one or more matches to this
value. This operation stops at first match
+ * but may be more expensive than the containsKey method.
+ * @since 2.0
Review Comment:
Wrong `since` tag.
##########
src/test/java/org/apache/commons/configuration2/TestAbstractConfiguration.java:
##########
@@ -78,6 +78,35 @@ public void testAddPropertyDirect() {
assertEquals(expected, list);
}
+ @Test
+ public void testContainsValue(){
+ final Configuration config = getConfiguration();
+ assertTrue(config.containsValue("value1"), "should return true for
class " + this.getClass());
+ assertFalse(config.containsValue("value99999"), "should return false
for class " + this.getClass());
+ }
+
+ @Test
+ public void testContains(){
+ final AbstractConfiguration config = getConfiguration();
+ assertTrue(config.contains(config.getKeys(), "value1"), "should return
true for class " + this.getClass());
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/TestDataConfiguration.java:
##########
@@ -1985,4 +1985,10 @@ public void testIsEmpty() {
public void testNullConfiguration() {
assertThrows(NullPointerException.class, () -> new
DataConfiguration(null));
}
+
+ @Test
+ public void givenNullValue_testContainsValue_shouldReturnFalse() {
+ final Configuration config = conf.getConfiguration();
+ assertFalse(config.containsValue(null), "should return false");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/TestAbstractHierarchicalConfiguration.java:
##########
@@ -941,4 +941,9 @@ public void testSetProperty() {
public void testSize() {
assertEquals(2, config.size());
}
+
+ @Test
+ public void
givenNullValue_testContainsValue_shouldThrowNullPointerException() {
+ assertThrows(NullPointerException.class, () ->
config.containsValue(null), "should throw NullPointerException");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/web/TestServletConfiguration.java:
##########
@@ -97,4 +98,10 @@ public void testAddPropertyDirect() {
public void testClearProperty() {
assertThrows(UnsupportedOperationException.class,
super::testClearProperty);
}
+
+ @Test
+ public void
givenNullValue_testContainsValue_shouldThrowNullPointerException() {
+ final Configuration config = getConfiguration();
+ assertThrows(NullPointerException.class, () ->
config.containsValue(null), "should throw NullPointerException");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/TestMapConfiguration.java:
##########
@@ -162,4 +162,10 @@ public void testNullMap() {
assertThrows(NullPointerException.class, () -> new
MapConfiguration((Map) null));
assertThrows(NullPointerException.class, () -> new
MapConfiguration((Properties) null));
}
+
+ @Test
+ public void givenNullValue_testContainsValue_shouldReturnFalse() {
+ final Configuration config = getConfiguration();
Review Comment:
Inline single-use local variable.
##########
src/test/java/org/apache/commons/configuration2/web/TestAppletConfiguration.java:
##########
@@ -112,4 +113,10 @@ public void testClearProperty() {
assertThrows(UnsupportedOperationException.class,
super::testClearProperty);
}
}
+
+ @Test
+ public void
givenNullValue_testContainsValue_shouldThrowNullPointerException() {
+ final Configuration config = getConfiguration();
+ assertThrows(NullPointerException.class, () ->
config.containsValue(null), "should throw NullPointerException");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/web/TestServletContextConfiguration.java:
##########
@@ -107,4 +108,10 @@ public void testAddPropertyDirect() {
public void testClearProperty() {
assertThrows(UnsupportedOperationException.class,
super::testClearProperty);
}
+
+ @Test
+ public void
givenNullValue_testContainsValue_shouldThrowNullPointerException() {
+ final Configuration config = getConfiguration();
+ assertThrows(NullPointerException.class, () ->
config.containsValue(null), "should throw NullPointerException");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/TestAbstractConfiguration.java:
##########
@@ -78,6 +78,35 @@ public void testAddPropertyDirect() {
assertEquals(expected, list);
}
+ @Test
+ public void testContainsValue(){
+ final Configuration config = getConfiguration();
+ assertTrue(config.containsValue("value1"), "should return true for
class " + this.getClass());
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java:
##########
@@ -1212,6 +1213,15 @@ public void testNewLineEscaping() {
assertEquals(Arrays.asList("C:\\path1\\", "C:\\path2\\",
"C:\\path3\\complex\\test\\"), list);
}
+ @Test
+ void testConfiguration() throws ConfigurationException {
+ Configurations configManager = new Configurations();
+ Configuration config =
configManager.properties("src/test/resources/config/test.properties");
+
+ assertTrue(config.containsValue("jndivalue2"), "should return true");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
##########
src/test/java/org/apache/commons/configuration2/web/TestServletRequestConfiguration.java:
##########
@@ -115,4 +115,10 @@ public void testListWithEscapedElements() {
}
assertEquals(expected, v);
}
+
+ @Test
+ public void
givenNullValue_testContainsValue_shouldThrowNullPointerException() {
+ final Configuration config = getConfiguration();
+ assertThrows(NullPointerException.class, () ->
config.containsValue(null), "should throw NullPointerException");
Review Comment:
No need for this message strings, it just clutters up the code and is
redundant with what JUnit provides.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
