Copilot commented on code in PR #7721:
URL: https://github.com/apache/incubator-seata/pull/7721#discussion_r2513838719
##########
common/src/test/java/org/apache/seata/common/util/SizeUtilTest.java:
##########
@@ -36,4 +36,98 @@ void size2Long() {
assertThat(SizeUtil.size2Long("2G")).isEqualTo(2L * 1024 * 1024 *
1024);
assertThat(SizeUtil.size2Long("2t")).isEqualTo(2L * 1024 * 1024 * 1024
* 1024);
}
+
+ @Test
+ public void testSize2LongWithValidInputs() {
+ // Test all valid units in lowercase
+ assertThat(SizeUtil.size2Long("1k")).isEqualTo(1L * 1024);
+ assertThat(SizeUtil.size2Long("1m")).isEqualTo(1L * 1024 * 1024);
+ assertThat(SizeUtil.size2Long("1g")).isEqualTo(1L * 1024 * 1024 *
1024);
+ assertThat(SizeUtil.size2Long("1t")).isEqualTo(1L * 1024 * 1024 * 1024
* 1024);
+
+ // Test all valid units in uppercase
+ assertThat(SizeUtil.size2Long("1K")).isEqualTo(1L * 1024);
+ assertThat(SizeUtil.size2Long("1M")).isEqualTo(1L * 1024 * 1024);
+ assertThat(SizeUtil.size2Long("1G")).isEqualTo(1L * 1024 * 1024 *
1024);
+ assertThat(SizeUtil.size2Long("1T")).isEqualTo(1L * 1024 * 1024 * 1024
* 1024);
+
+ // Test larger numbers
+ assertThat(SizeUtil.size2Long("1024k")).isEqualTo(1024L * 1024);
+ assertThat(SizeUtil.size2Long("512m")).isEqualTo(512L * 1024 * 1024);
+ assertThat(SizeUtil.size2Long("2g")).isEqualTo(2L * 1024 * 1024 *
1024);
+ assertThat(SizeUtil.size2Long("1t")).isEqualTo(1L * 1024 * 1024 * 1024
* 1024);
+
+ // Test single digit numbers
+ assertThat(SizeUtil.size2Long("0k")).isEqualTo(0L);
+ assertThat(SizeUtil.size2Long("5m")).isEqualTo(5L * 1024 * 1024);
+ }
+
+ @Test
+ public void testSize2LongWithInvalidInputs() {
+ // Test null input
+ assertThatThrownBy(() -> SizeUtil.size2Long(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert 'null' to byte
length");
+
+ // Test empty string
+ assertThatThrownBy(() -> SizeUtil.size2Long(""))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '' to byte length");
+
+ // Test single character
+ assertThatThrownBy(() -> SizeUtil.size2Long("k"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert 'k' to byte length");
+
+ // Test invalid number format
+ assertThatThrownBy(() -> SizeUtil.size2Long("ak"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert 'ak' to byte length");
+
+ // Test invalid unit
+ assertThatThrownBy(() -> SizeUtil.size2Long("1x"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '1x' to byte length");
+
+ // Test negative numbers
+ assertThat(SizeUtil.size2Long("-1k")).isEqualTo(-1L * 1024);
Review Comment:
The comment is misleading. The test on line 93 checks that negative numbers
are handled correctly (they work), but this test is in a method called
"testSize2LongWithInvalidInputs" which suggests these should fail. Either move
lines 92-96 to the valid inputs test or remove them from this test method as
they represent valid behavior.
##########
common/src/main/java/org/apache/seata/common/util/IOUtil.java:
##########
@@ -31,14 +38,15 @@ public static void close(AutoCloseable... closeables) {
}
/**
- * close Closeable
+ * Close Closeable resource
* @param closeable the closeable
*/
public static void close(AutoCloseable closeable) {
if (closeable != null) {
try {
closeable.close();
- } catch (Exception ignore) {
+ } catch (Exception e) {
+ LOGGER.warn("Failed to close resource", e);
Review Comment:
[nitpick] Changing from swallowing exceptions silently to logging them is
good, but this changes the behavior of the `close` method. Previously,
exceptions during close were silently ignored, which is a common pattern for
cleanup operations. Now logging warnings may cause log spam in production.
Consider using a lower log level like `debug` or `trace` for close failures, or
add a configuration option to control this behavior.
```suggestion
LOGGER.debug("Failed to close resource", e);
```
##########
common/src/test/java/org/apache/seata/common/util/CollectionUtilsTest.java:
##########
@@ -255,4 +258,335 @@ public void test_getLast() {
list.add("Bar");
Assertions.assertEquals("Bar", CollectionUtils.getLast(list));
}
+
+ @Test
+ public void testComputeIfAbsent() {
+ // Test with a regular HashMap
+ Map<String, String> map = new HashMap<>();
+ map.put("existing", "value");
+
+ // Test existing key - should return existing value and not call the
function
+ Function<String, String> mappingFunction = key -> {
+ throw new RuntimeException("Function should not be called for
existing key");
+ };
+ String result = CollectionUtils.computeIfAbsent(map, "existing",
mappingFunction);
+ assertThat(result).isEqualTo("value");
+
+ // Test non-existing key - should call the function and add the value
+ Function<String, String> newMappingFunction = key -> "new value for "
+ key;
+ String newResult = CollectionUtils.computeIfAbsent(map, "newKey",
newMappingFunction);
+ assertThat(newResult).isEqualTo("new value for newKey");
+ assertThat(map.get("newKey")).isEqualTo("new value for newKey");
+
+ // Test with null value for existing key
+ Map<String, String> mapWithNull = new HashMap<>();
+ mapWithNull.put("nullKey", null);
+ Function<String, String> nullMappingFunction = key -> "new value for
null";
+ String nullResult = CollectionUtils.computeIfAbsent(mapWithNull,
"nullKey", nullMappingFunction);
+ assertThat(nullResult).isEqualTo("new value for null");
+ assertThat(mapWithNull.get("nullKey")).isEqualTo("new value for null");
+ }
+
+ @Test
+ public void testMapToJsonString() {
+ // Test with empty map
+ Map<String, Object> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.mapToJsonString(emptyMap)).isEqualTo("{}");
+
+ // Test with simple key-value pairs
+ Map<String, Object> simpleMap = new HashMap<>();
+ simpleMap.put("name", "John");
+ simpleMap.put("age", 30);
+ String simpleResult = CollectionUtils.mapToJsonString(simpleMap);
+ assertThat(simpleResult).contains("\"name\": \"John\"");
+ assertThat(simpleResult).contains("\"age\": 30");
+
+ // Test with nested map - order might vary in HashMap, so we check
both possible orders
+ Map<String, Object> nestedMap = new HashMap<>();
+ nestedMap.put("id", 1);
+ Map<String, Object> address = new HashMap<>();
+ address.put("city", "New York");
+ address.put("zip", "10001");
+ nestedMap.put("address", address);
+ String result = CollectionUtils.mapToJsonString(nestedMap);
+ // Check if result contains expected elements regardless of order
+ assertThat(result).contains("\"id\": 1");
+ assertThat(result).contains("\"city\": \"New York\"");
+ assertThat(result).contains("\"zip\": \"10001\"");
+
+ // Test with null values
+ Map<String, Object> mapWithNull = new HashMap<>();
+ mapWithNull.put("key1", "value1");
+ mapWithNull.put("key2", null);
+ String nullResult = CollectionUtils.mapToJsonString(mapWithNull);
+ assertThat(nullResult).contains("\"key1\": \"value1\"");
+ assertThat(nullResult).contains("\"key2\": null");
+
+ // Test with special characters in strings
+ Map<String, Object> mapWithSpecialChars = new HashMap<>();
+ mapWithSpecialChars.put("quote", "\"quoted\"");
+ mapWithSpecialChars.put("number", 42);
+ String specialResult =
CollectionUtils.mapToJsonString(mapWithSpecialChars);
+ // We'll just check that it contains the expected elements without
strict formatting
+ assertThat(specialResult).contains("\"quote\"");
+ assertThat(specialResult).contains("\"number\"");
+ assertThat(specialResult).contains("42");
+ }
+
+ @Test
+ public void testToStringMapEnhanced() {
+ // Test with null map
+ Map<String, String> nullResult = CollectionUtils.toStringMap(null);
+ assertThat(nullResult).isNotNull().isEmpty();
+
+ // Test with empty map
+ Map<String, Object> emptySource = new HashMap<>();
+ Map<String, String> emptyResult =
CollectionUtils.toStringMap(emptySource);
+ assertThat(emptyResult).isNotNull().isEmpty();
+
+ // Test with various object types
+ Map<String, Object> sourceMap = new HashMap<>();
+ sourceMap.put("string", "text");
+ sourceMap.put("integer", 123);
+ sourceMap.put("double", 45.67);
+ sourceMap.put("boolean", true);
+ sourceMap.put("nullValue", null);
+
+ Map<String, String> result = CollectionUtils.toStringMap(sourceMap);
+ assertThat(result)
+ .containsEntry("string", "text")
+ .containsEntry("integer", "123")
+ .containsEntry("double", "45.67")
+ .containsEntry("boolean", "true")
+ .doesNotContainKey("nullValue"); // null values should be
skipped
+
+ // Test with CharSequence and Character
+ Map<String, Object> charMap = new HashMap<>();
+ charMap.put("charSequence", new StringBuilder("builder"));
+ charMap.put("character", 'A');
+
+ Map<String, String> charResult = CollectionUtils.toStringMap(charMap);
+ assertThat(charResult).containsEntry("charSequence",
"builder").containsEntry("character", "A");
+ }
+
+ @Test
+ public void testGetLastWithConcurrentModification() {
+ // Test with normal list
+ List<String> list = new ArrayList<>();
+ list.add("first");
+ list.add("second");
+ list.add("third");
+ assertThat((String)
CollectionUtils.<String>getLast(list)).isEqualTo("third");
+
+ // Test with empty list
+ List<String> emptyList = new ArrayList<>();
+ assertThat(CollectionUtils.<String>getLast(emptyList)).isNull();
+
+ // Test with null list
+ assertThat(CollectionUtils.<String>getLast(null)).isNull();
+ }
+
+ @Test
+ public void testIsEmptyWithArraysEnhanced() {
+ // Test with null array
+ String[] nullArray = null;
+ assertThat(CollectionUtils.isEmpty(nullArray)).isTrue();
+
+ // Test with empty array
+ String[] emptyArray = new String[0];
+ assertThat(CollectionUtils.isEmpty(emptyArray)).isTrue();
+
+ // Test with filled array
+ String[] filledArray = {"one", "two"};
+ assertThat(CollectionUtils.isEmpty(filledArray)).isFalse();
+ }
+
+ @Test
+ public void testIsNotEmptyWithArrays() {
+ // Test with null array
+ String[] nullArray = null;
+ assertThat(CollectionUtils.isNotEmpty(nullArray)).isFalse();
+
+ // Test with empty array
+ String[] emptyArray = new String[0];
+ assertThat(CollectionUtils.isNotEmpty(emptyArray)).isFalse();
+
+ // Test with filled array
+ String[] filledArray = {"one", "two"};
+ assertThat(CollectionUtils.isNotEmpty(filledArray)).isTrue();
+ }
+
+ @Test
+ public void testIsEmptyWithCollections() {
+ // Test with null collection
+ List<String> nullList = null;
+ assertThat(CollectionUtils.isEmpty(nullList)).isTrue();
+
+ // Test with empty collection
+ List<String> emptyList = new ArrayList<>();
+ assertThat(CollectionUtils.isEmpty(emptyList)).isTrue();
+
+ // Test with filled collection
+ List<String> filledList = new ArrayList<>();
+ filledList.add("item");
+ assertThat(CollectionUtils.isEmpty(filledList)).isFalse();
+ }
+
+ @Test
+ public void testIsNotEmptyWithCollections() {
+ // Test with null collection
+ List<String> nullList = null;
+ assertThat(CollectionUtils.isNotEmpty(nullList)).isFalse();
+
+ // Test with empty collection
+ List<String> emptyList = new ArrayList<>();
+ assertThat(CollectionUtils.isNotEmpty(emptyList)).isFalse();
+
+ // Test with filled collection
+ List<String> filledList = new ArrayList<>();
+ filledList.add("item");
+ assertThat(CollectionUtils.isNotEmpty(filledList)).isTrue();
+ }
+
+ @Test
+ public void testIsEmptyWithMaps() {
+ // Test with null map
+ Map<String, String> nullMap = null;
+ assertThat(CollectionUtils.isEmpty(nullMap)).isTrue();
+
+ // Test with empty map
+ Map<String, String> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.isEmpty(emptyMap)).isTrue();
+
+ // Test with filled map
+ Map<String, String> filledMap = new HashMap<>();
+ filledMap.put("key", "value");
+ assertThat(CollectionUtils.isEmpty(filledMap)).isFalse();
+ }
+
+ @Test
+ public void testIsNotEmptyWithMaps() {
+ // Test with null map
+ Map<String, String> nullMap = null;
+ assertThat(CollectionUtils.isNotEmpty(nullMap)).isFalse();
+
+ // Test with empty map
+ Map<String, String> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.isNotEmpty(emptyMap)).isFalse();
+
+ // Test with filled map
+ Map<String, String> filledMap = new HashMap<>();
+ filledMap.put("key", "value");
+ assertThat(CollectionUtils.isNotEmpty(filledMap)).isTrue();
+ }
+
+ @Test
+ public void testEncodeMapEnhanced() {
+ // Test with null map
+ assertThat(CollectionUtils.encodeMap(null)).isNull();
+
+ // Test with empty map
+ Map<String, String> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.encodeMap(emptyMap)).isEqualTo("");
+
+ // Test with single entry
+ Map<String, String> singleEntry = new HashMap<>();
+ singleEntry.put("key", "value");
+
assertThat(CollectionUtils.encodeMap(singleEntry)).isEqualTo("key=value");
+
+ // Test with multiple entries
+ Map<String, String> multipleEntries = new HashMap<>();
+ multipleEntries.put("key1", "value1");
+ multipleEntries.put("key2", "value2");
+
assertThat(CollectionUtils.encodeMap(multipleEntries)).isEqualTo("key1=value1&key2=value2");
+
+ // Test with special characters
+ Map<String, String> specialChars = new HashMap<>();
+ specialChars.put("key with spaces", "value&special=chars");
+ assertThat(CollectionUtils.encodeMap(specialChars)).isEqualTo("key
with spaces=value&special=chars");
+ }
+
+ @Test
+ public void testDecodeMapEnhanced() {
+ // Test with null input
+ assertThat(CollectionUtils.decodeMap(null)).isNull();
+
+ // Test with empty string
+ Map<String, String> emptyResult = CollectionUtils.decodeMap("");
+ assertThat(emptyResult).isNotNull().isEmpty();
+
+ // Test with single pair
+ Map<String, String> singleResult =
CollectionUtils.decodeMap("key=value");
+ assertThat(singleResult).containsEntry("key", "value");
+
+ // Test with multiple pairs
+ Map<String, String> multipleResult =
CollectionUtils.decodeMap("key1=value1&key2=value2");
+ assertThat(multipleResult).containsEntry("key1",
"value1").containsEntry("key2", "value2");
+
+ // Test with malformed pairs (should be ignored)
+ Map<String, String> malformedResult =
CollectionUtils.decodeMap("key1=value1&malformed&key2=value2");
+ assertThat(malformedResult)
+ .containsEntry("key1", "value1")
+ .containsEntry("key2", "value2")
+ .hasSize(2);
+
+ // Test with empty keys or values
+ Map<String, String> emptyKVResult =
CollectionUtils.decodeMap("key1=&=value2&key3=value3");
+ // The entry with empty key results in key "" with value "value2"
+ assertThat(emptyKVResult)
+ .containsEntry("", "value2")
+ .containsEntry("key3", "value3")
+ .hasSize(2);
Review Comment:
The test assertion on line 539 is incorrect. Looking at the decode logic,
when parsing "key1=&=value2", the entry "key1=" will split into ["key1", ""],
and the entry "=value2" will split into ["", "value2"]. However, the test
expects only 2 entries with keys "" and "key3", which would mean "key1" is
missing. The actual result should have 3 entries: ("key1", ""), ("", "value2"),
and ("key3", "value3").
```suggestion
.containsEntry("key1", "")
.containsEntry("", "value2")
.containsEntry("key3", "value3")
.hasSize(3);
```
##########
common/src/test/java/org/apache/seata/common/util/SizeUtilTest.java:
##########
@@ -36,4 +36,98 @@ void size2Long() {
assertThat(SizeUtil.size2Long("2G")).isEqualTo(2L * 1024 * 1024 *
1024);
assertThat(SizeUtil.size2Long("2t")).isEqualTo(2L * 1024 * 1024 * 1024
* 1024);
}
+
+ @Test
+ public void testSize2LongWithValidInputs() {
+ // Test all valid units in lowercase
+ assertThat(SizeUtil.size2Long("1k")).isEqualTo(1L * 1024);
+ assertThat(SizeUtil.size2Long("1m")).isEqualTo(1L * 1024 * 1024);
+ assertThat(SizeUtil.size2Long("1g")).isEqualTo(1L * 1024 * 1024 *
1024);
+ assertThat(SizeUtil.size2Long("1t")).isEqualTo(1L * 1024 * 1024 * 1024
* 1024);
+
+ // Test all valid units in uppercase
+ assertThat(SizeUtil.size2Long("1K")).isEqualTo(1L * 1024);
+ assertThat(SizeUtil.size2Long("1M")).isEqualTo(1L * 1024 * 1024);
+ assertThat(SizeUtil.size2Long("1G")).isEqualTo(1L * 1024 * 1024 *
1024);
+ assertThat(SizeUtil.size2Long("1T")).isEqualTo(1L * 1024 * 1024 * 1024
* 1024);
+
+ // Test larger numbers
+ assertThat(SizeUtil.size2Long("1024k")).isEqualTo(1024L * 1024);
+ assertThat(SizeUtil.size2Long("512m")).isEqualTo(512L * 1024 * 1024);
+ assertThat(SizeUtil.size2Long("2g")).isEqualTo(2L * 1024 * 1024 *
1024);
+ assertThat(SizeUtil.size2Long("1t")).isEqualTo(1L * 1024 * 1024 * 1024
* 1024);
+
+ // Test single digit numbers
+ assertThat(SizeUtil.size2Long("0k")).isEqualTo(0L);
+ assertThat(SizeUtil.size2Long("5m")).isEqualTo(5L * 1024 * 1024);
+ }
+
+ @Test
+ public void testSize2LongWithInvalidInputs() {
+ // Test null input
+ assertThatThrownBy(() -> SizeUtil.size2Long(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert 'null' to byte
length");
+
+ // Test empty string
+ assertThatThrownBy(() -> SizeUtil.size2Long(""))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '' to byte length");
+
+ // Test single character
+ assertThatThrownBy(() -> SizeUtil.size2Long("k"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert 'k' to byte length");
+
+ // Test invalid number format
+ assertThatThrownBy(() -> SizeUtil.size2Long("ak"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert 'ak' to byte length");
+
+ // Test invalid unit
+ assertThatThrownBy(() -> SizeUtil.size2Long("1x"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '1x' to byte length");
+
+ // Test negative numbers
+ assertThat(SizeUtil.size2Long("-1k")).isEqualTo(-1L * 1024);
+
+ // Test zero
+ assertThat(SizeUtil.size2Long("0g")).isEqualTo(0L);
+
+ // Test invalid format with multiple characters
+ assertThatThrownBy(() -> SizeUtil.size2Long("12kk"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '12kk' to byte
length");
+
+ // Test decimal numbers (should be truncated)
+ assertThatThrownBy(() -> SizeUtil.size2Long("1.5k"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '1.5k' to byte
length");
+ }
+
+ @Test
+ public void testSize2LongWithBoundaryConditions() {
+ // Test maximum values that don't overflow
+ assertThat(SizeUtil.size2Long("8k")).isEqualTo(8L * 1024);
+
+ // Test large valid values
+ assertThat(SizeUtil.size2Long("1000000k")).isEqualTo(1000000L * 1024);
+
+ // Test with spaces (should be invalid)
+ assertThatThrownBy(() -> SizeUtil.size2Long("1 k"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("could not convert '1 k' to byte
length");
+ }
+
+ @Test
+ public void testSize2LongWithSpecialCases() {
+ // Test that we handle long numbers properly
+ String largeNumber = Long.toString(Long.MAX_VALUE / (1024 * 1024 *
1024));
+ // This should work without overflow
+ assertThat(SizeUtil.size2Long(largeNumber +
"g")).isEqualTo(Long.parseLong(largeNumber) * 1024 * 1024 * 1024);
Review Comment:
Potential uncaught 'java.lang.NumberFormatException'.
##########
common/src/test/java/org/apache/seata/common/util/ReflectionUtilTest.java:
##########
@@ -234,6 +462,56 @@ public void setF2(String f2) {
interface TestInterface {}
+ // Enhanced test classes and interfaces
+ interface TestInterfaceEnhanced {
+ void interfaceMethod();
+ }
+
+ interface SecondInterface {
+ void secondMethod();
+ }
+
+ static class TestSuperClassEnhanced {
+ private Integer f2;
+
+ public Integer getF2() {
+ return f2;
+ }
+
+ public void setF2(Integer f2) {
+ this.f2 = f2;
+ }
+ }
+
+ static class TestClassEnhanced extends TestSuperClassEnhanced {
+ private String f1;
+
+ public String getValue() {
+ return f1;
+ }
+
+ public void setValue(String value) {
+ this.f1 = value;
+ }
+ }
+
+ static class MultiInterfaceImpl implements TestInterfaceEnhanced,
SecondInterface {
+ @Override
+ public void interfaceMethod() {}
+
+ @Override
+ public void secondMethod() {}
+ }
+
+ static class TestImpl implements TestInterfaceEnhanced {
+ @Override
+ public void interfaceMethod() {}
+ }
+
+ static class TestConstants {
Review Comment:
Unused class: TestConstants is not referenced within this codebase. If not
used as an external API it should be removed.
##########
common/src/test/java/org/apache/seata/common/util/CollectionUtilsTest.java:
##########
@@ -255,4 +258,335 @@ public void test_getLast() {
list.add("Bar");
Assertions.assertEquals("Bar", CollectionUtils.getLast(list));
}
+
+ @Test
+ public void testComputeIfAbsent() {
+ // Test with a regular HashMap
+ Map<String, String> map = new HashMap<>();
+ map.put("existing", "value");
+
+ // Test existing key - should return existing value and not call the
function
+ Function<String, String> mappingFunction = key -> {
+ throw new RuntimeException("Function should not be called for
existing key");
+ };
+ String result = CollectionUtils.computeIfAbsent(map, "existing",
mappingFunction);
+ assertThat(result).isEqualTo("value");
+
+ // Test non-existing key - should call the function and add the value
+ Function<String, String> newMappingFunction = key -> "new value for "
+ key;
+ String newResult = CollectionUtils.computeIfAbsent(map, "newKey",
newMappingFunction);
+ assertThat(newResult).isEqualTo("new value for newKey");
+ assertThat(map.get("newKey")).isEqualTo("new value for newKey");
+
+ // Test with null value for existing key
+ Map<String, String> mapWithNull = new HashMap<>();
+ mapWithNull.put("nullKey", null);
+ Function<String, String> nullMappingFunction = key -> "new value for
null";
+ String nullResult = CollectionUtils.computeIfAbsent(mapWithNull,
"nullKey", nullMappingFunction);
+ assertThat(nullResult).isEqualTo("new value for null");
+ assertThat(mapWithNull.get("nullKey")).isEqualTo("new value for null");
+ }
+
+ @Test
+ public void testMapToJsonString() {
+ // Test with empty map
+ Map<String, Object> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.mapToJsonString(emptyMap)).isEqualTo("{}");
+
+ // Test with simple key-value pairs
+ Map<String, Object> simpleMap = new HashMap<>();
+ simpleMap.put("name", "John");
+ simpleMap.put("age", 30);
+ String simpleResult = CollectionUtils.mapToJsonString(simpleMap);
+ assertThat(simpleResult).contains("\"name\": \"John\"");
+ assertThat(simpleResult).contains("\"age\": 30");
+
+ // Test with nested map - order might vary in HashMap, so we check
both possible orders
+ Map<String, Object> nestedMap = new HashMap<>();
+ nestedMap.put("id", 1);
+ Map<String, Object> address = new HashMap<>();
+ address.put("city", "New York");
+ address.put("zip", "10001");
+ nestedMap.put("address", address);
+ String result = CollectionUtils.mapToJsonString(nestedMap);
+ // Check if result contains expected elements regardless of order
+ assertThat(result).contains("\"id\": 1");
+ assertThat(result).contains("\"city\": \"New York\"");
+ assertThat(result).contains("\"zip\": \"10001\"");
+
+ // Test with null values
+ Map<String, Object> mapWithNull = new HashMap<>();
+ mapWithNull.put("key1", "value1");
+ mapWithNull.put("key2", null);
+ String nullResult = CollectionUtils.mapToJsonString(mapWithNull);
+ assertThat(nullResult).contains("\"key1\": \"value1\"");
+ assertThat(nullResult).contains("\"key2\": null");
+
+ // Test with special characters in strings
+ Map<String, Object> mapWithSpecialChars = new HashMap<>();
+ mapWithSpecialChars.put("quote", "\"quoted\"");
+ mapWithSpecialChars.put("number", 42);
+ String specialResult =
CollectionUtils.mapToJsonString(mapWithSpecialChars);
+ // We'll just check that it contains the expected elements without
strict formatting
+ assertThat(specialResult).contains("\"quote\"");
+ assertThat(specialResult).contains("\"number\"");
+ assertThat(specialResult).contains("42");
+ }
+
+ @Test
+ public void testToStringMapEnhanced() {
+ // Test with null map
+ Map<String, String> nullResult = CollectionUtils.toStringMap(null);
+ assertThat(nullResult).isNotNull().isEmpty();
+
+ // Test with empty map
+ Map<String, Object> emptySource = new HashMap<>();
+ Map<String, String> emptyResult =
CollectionUtils.toStringMap(emptySource);
+ assertThat(emptyResult).isNotNull().isEmpty();
+
+ // Test with various object types
+ Map<String, Object> sourceMap = new HashMap<>();
+ sourceMap.put("string", "text");
+ sourceMap.put("integer", 123);
+ sourceMap.put("double", 45.67);
+ sourceMap.put("boolean", true);
+ sourceMap.put("nullValue", null);
+
+ Map<String, String> result = CollectionUtils.toStringMap(sourceMap);
+ assertThat(result)
+ .containsEntry("string", "text")
+ .containsEntry("integer", "123")
+ .containsEntry("double", "45.67")
+ .containsEntry("boolean", "true")
+ .doesNotContainKey("nullValue"); // null values should be
skipped
+
+ // Test with CharSequence and Character
+ Map<String, Object> charMap = new HashMap<>();
+ charMap.put("charSequence", new StringBuilder("builder"));
+ charMap.put("character", 'A');
+
+ Map<String, String> charResult = CollectionUtils.toStringMap(charMap);
+ assertThat(charResult).containsEntry("charSequence",
"builder").containsEntry("character", "A");
+ }
+
+ @Test
+ public void testGetLastWithConcurrentModification() {
+ // Test with normal list
+ List<String> list = new ArrayList<>();
+ list.add("first");
+ list.add("second");
+ list.add("third");
+ assertThat((String)
CollectionUtils.<String>getLast(list)).isEqualTo("third");
+
+ // Test with empty list
+ List<String> emptyList = new ArrayList<>();
+ assertThat(CollectionUtils.<String>getLast(emptyList)).isNull();
+
+ // Test with null list
+ assertThat(CollectionUtils.<String>getLast(null)).isNull();
+ }
+
+ @Test
+ public void testIsEmptyWithArraysEnhanced() {
+ // Test with null array
+ String[] nullArray = null;
+ assertThat(CollectionUtils.isEmpty(nullArray)).isTrue();
+
+ // Test with empty array
+ String[] emptyArray = new String[0];
+ assertThat(CollectionUtils.isEmpty(emptyArray)).isTrue();
+
+ // Test with filled array
+ String[] filledArray = {"one", "two"};
+ assertThat(CollectionUtils.isEmpty(filledArray)).isFalse();
+ }
+
+ @Test
+ public void testIsNotEmptyWithArrays() {
+ // Test with null array
+ String[] nullArray = null;
+ assertThat(CollectionUtils.isNotEmpty(nullArray)).isFalse();
+
+ // Test with empty array
+ String[] emptyArray = new String[0];
+ assertThat(CollectionUtils.isNotEmpty(emptyArray)).isFalse();
+
+ // Test with filled array
+ String[] filledArray = {"one", "two"};
+ assertThat(CollectionUtils.isNotEmpty(filledArray)).isTrue();
+ }
+
+ @Test
+ public void testIsEmptyWithCollections() {
+ // Test with null collection
+ List<String> nullList = null;
+ assertThat(CollectionUtils.isEmpty(nullList)).isTrue();
+
+ // Test with empty collection
+ List<String> emptyList = new ArrayList<>();
+ assertThat(CollectionUtils.isEmpty(emptyList)).isTrue();
+
+ // Test with filled collection
+ List<String> filledList = new ArrayList<>();
+ filledList.add("item");
+ assertThat(CollectionUtils.isEmpty(filledList)).isFalse();
+ }
+
+ @Test
+ public void testIsNotEmptyWithCollections() {
+ // Test with null collection
+ List<String> nullList = null;
+ assertThat(CollectionUtils.isNotEmpty(nullList)).isFalse();
+
+ // Test with empty collection
+ List<String> emptyList = new ArrayList<>();
+ assertThat(CollectionUtils.isNotEmpty(emptyList)).isFalse();
+
+ // Test with filled collection
+ List<String> filledList = new ArrayList<>();
+ filledList.add("item");
+ assertThat(CollectionUtils.isNotEmpty(filledList)).isTrue();
+ }
+
+ @Test
+ public void testIsEmptyWithMaps() {
+ // Test with null map
+ Map<String, String> nullMap = null;
+ assertThat(CollectionUtils.isEmpty(nullMap)).isTrue();
+
+ // Test with empty map
+ Map<String, String> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.isEmpty(emptyMap)).isTrue();
+
+ // Test with filled map
+ Map<String, String> filledMap = new HashMap<>();
+ filledMap.put("key", "value");
+ assertThat(CollectionUtils.isEmpty(filledMap)).isFalse();
+ }
+
+ @Test
+ public void testIsNotEmptyWithMaps() {
+ // Test with null map
+ Map<String, String> nullMap = null;
+ assertThat(CollectionUtils.isNotEmpty(nullMap)).isFalse();
+
+ // Test with empty map
+ Map<String, String> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.isNotEmpty(emptyMap)).isFalse();
+
+ // Test with filled map
+ Map<String, String> filledMap = new HashMap<>();
+ filledMap.put("key", "value");
+ assertThat(CollectionUtils.isNotEmpty(filledMap)).isTrue();
+ }
+
+ @Test
+ public void testEncodeMapEnhanced() {
+ // Test with null map
+ assertThat(CollectionUtils.encodeMap(null)).isNull();
+
+ // Test with empty map
+ Map<String, String> emptyMap = new HashMap<>();
+ assertThat(CollectionUtils.encodeMap(emptyMap)).isEqualTo("");
+
+ // Test with single entry
+ Map<String, String> singleEntry = new HashMap<>();
+ singleEntry.put("key", "value");
+
assertThat(CollectionUtils.encodeMap(singleEntry)).isEqualTo("key=value");
+
+ // Test with multiple entries
+ Map<String, String> multipleEntries = new HashMap<>();
Review Comment:
The test assertion on line 501 is brittle and will fail
non-deterministically. `HashMap` does not guarantee insertion order, so the
encoded string could be either "key1=value1&key2=value2" or
"key2=value2&key1=value1". Use `LinkedHashMap` instead of `HashMap` for
predictable ordering, or change the assertion to check that both key-value
pairs are present without relying on order.
```suggestion
Map<String, String> multipleEntries = new LinkedHashMap<>();
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
