This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-collections.git
The following commit(s) were added to refs/heads/master by this push: new 7d06bd7 [COLLECTIONS-747] MultiKey.getKeys class cast exception. 7d06bd7 is described below commit 7d06bd77e9ebda68c79eac20075560d488a28364 Author: Gary Gregory <garydgreg...@gmail.com> AuthorDate: Sun Feb 16 15:18:27 2020 -0500 [COLLECTIONS-747] MultiKey.getKeys class cast exception. --- src/changes/changes.xml | 3 + .../commons/collections4/keyvalue/MultiKey.java | 192 +++++++++++++-------- .../collections4/keyvalue/MultiKeyTest.java | 12 +- 3 files changed, 136 insertions(+), 71 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index ead842a..5ad5517 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -129,6 +129,9 @@ <action dev="ggregory" type="update" due-to="Gary Gregory"> [test] Update org.easymock:easymock 4.1 -> 4.2. </action> + <action issue="COLLECTIONS-747" dev="ggregory" type="fix" due-to="Gary Gregory, Walter Laan"> + MultiKey.getKeys class cast exception. + </action> </release> <release version="4.4" date="2019-07-05" description="Maintenance release."> <action issue="COLLECTIONS-710" dev="ggregory" type="fix" due-to="Yu Shi, Gary Gregory"> diff --git a/src/main/java/org/apache/commons/collections4/keyvalue/MultiKey.java b/src/main/java/org/apache/commons/collections4/keyvalue/MultiKey.java index 1261a06..c1080f8 100644 --- a/src/main/java/org/apache/commons/collections4/keyvalue/MultiKey.java +++ b/src/main/java/org/apache/commons/collections4/keyvalue/MultiKey.java @@ -17,6 +17,7 @@ package org.apache.commons.collections4.keyvalue; import java.io.Serializable; +import java.lang.reflect.Array; import java.util.Arrays; import java.util.Objects; @@ -51,8 +52,69 @@ public class MultiKey<K> implements Serializable { /** Serialisation version */ private static final long serialVersionUID = 4465448607415788805L; + @SuppressWarnings("unchecked") + private static <T> Class<? extends T> getClass(final T value) { + return (Class<? extends T>) (value == null ? Object.class : value.getClass()); + } + + private static <T> Class<? extends T> getComponentType(final T... values) { + @SuppressWarnings("unchecked") + final Class<? extends T> rootClass = (Class<? extends T>) Object.class; + if (values == null) { + return rootClass; + } + Class<? extends T> prevClass = values.length > 0 ? getClass(values[0]) : rootClass; + for (int i = 1; i < values.length; i++) { + final Class<? extends T> classI = getClass(values[i]); + if (prevClass != classI) { + return rootClass; + } + prevClass = classI; + } + return prevClass; + } + + private static <T> T[] newArray(final T key1, final T key2) { + @SuppressWarnings("unchecked") + final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2), 2); + array[0] = key1; + array[1] = key2; + return array; + } + + private static <T> T[] newArray(final T key1, final T key2, final T key3) { + @SuppressWarnings("unchecked") + final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2, key3), 3); + array[0] = key1; + array[1] = key2; + array[2] = key3; + return array; + } + + private static <T> T[] newArray(final T key1, final T key2, final T key3, final T key4) { + @SuppressWarnings("unchecked") + final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2, key3, key4), 4); + array[0] = key1; + array[1] = key2; + array[2] = key3; + array[3] = key4; + return array; + } + + private static <T> T[] newArray(final T key1, final T key2, final T key3, final T key4, final T key5) { + @SuppressWarnings("unchecked") + final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2, key3, key4, key5), 5); + array[0] = key1; + array[1] = key2; + array[2] = key3; + array[3] = key4; + array[4] = key5; + return array; + } + /** The individual keys */ private final K[] keys; + /** The cached hashCode */ private transient int hashCode; @@ -65,11 +127,11 @@ public class MultiKey<K> implements Serializable { * @param key1 the first key * @param key2 the second key */ - @SuppressWarnings("unchecked") public MultiKey(final K key1, final K key2) { - this((K[]) new Object[] { key1, key2 }, false); + this(newArray(key1, key2), false); } + /** * Constructor taking three keys. * <p> @@ -80,9 +142,8 @@ public class MultiKey<K> implements Serializable { * @param key2 the second key * @param key3 the third key */ - @SuppressWarnings("unchecked") public MultiKey(final K key1, final K key2, final K key3) { - this((K[]) new Object[] {key1, key2, key3}, false); + this(newArray(key1, key2, key3), false); } /** @@ -96,9 +157,8 @@ public class MultiKey<K> implements Serializable { * @param key3 the third key * @param key4 the fourth key */ - @SuppressWarnings("unchecked") public MultiKey(final K key1, final K key2, final K key3, final K key4) { - this((K[]) new Object[] {key1, key2, key3, key4}, false); + this(newArray(key1, key2, key3, key4), false); } /** @@ -113,9 +173,8 @@ public class MultiKey<K> implements Serializable { * @param key4 the fourth key * @param key5 the fifth key */ - @SuppressWarnings("unchecked") public MultiKey(final K key1, final K key2, final K key3, final K key4, final K key5) { - this((K[]) new Object[] {key1, key2, key3, key4, key5}, false); + this(newArray(key1, key2, key3, key4, key5), false); } /** @@ -160,26 +219,45 @@ public class MultiKey<K> implements Serializable { public MultiKey(final K[] keys, final boolean makeClone) { super(); Objects.requireNonNull(keys, "keys"); - if (makeClone) { - this.keys = keys.clone(); - } else { - this.keys = keys; - } - + this.keys = makeClone ? keys.clone() : keys; calculateHashCode(keys); } + /** + * Calculate the hash code of the instance using the provided keys. + * @param keys the keys to calculate the hash code for + */ + private void calculateHashCode(final Object[] keys) + { + int total = 0; + for (final Object key : keys) { + if (key != null) { + total ^= key.hashCode(); + } + } + hashCode = total; + } + //----------------------------------------------------------------------- /** - * Gets a clone of the array of keys. + * Compares this object to another. * <p> - * The keys should be immutable - * If they are not then they must not be changed. + * To be equal, the other object must be a {@code MultiKey} with the + * same number of keys which are also equal. * - * @return the individual keys + * @param other the other object to compare to + * @return true if equal */ - public K[] getKeys() { - return keys.clone(); + @Override + public boolean equals(final Object other) { + if (other == this) { + return true; + } + if (other instanceof MultiKey) { + final MultiKey<?> otherMulti = (MultiKey<?>) other; + return Arrays.equals(keys, otherMulti.keys); + } + return false; } /** @@ -197,36 +275,17 @@ public class MultiKey<K> implements Serializable { return keys[index]; } - /** - * Gets the size of the list of keys. - * - * @return the size of the list of keys - * @since 3.1 - */ - public int size() { - return keys.length; - } - //----------------------------------------------------------------------- /** - * Compares this object to another. + * Gets a clone of the array of keys. * <p> - * To be equal, the other object must be a {@code MultiKey} with the - * same number of keys which are also equal. + * The keys should be immutable + * If they are not then they must not be changed. * - * @param other the other object to compare to - * @return true if equal + * @return the individual keys */ - @Override - public boolean equals(final Object other) { - if (other == this) { - return true; - } - if (other instanceof MultiKey) { - final MultiKey<?> otherMulti = (MultiKey<?>) other; - return Arrays.equals(keys, otherMulti.keys); - } - return false; + public K[] getKeys() { + return keys.clone(); } /** @@ -245,38 +304,33 @@ public class MultiKey<K> implements Serializable { } /** - * Gets a debugging string version of the key. - * - * @return a debugging string + * Recalculate the hash code after deserialization. The hash code of some + * keys might have change (hash codes based on the system hash code are + * only stable for the same process). + * @return the instance with recalculated hash code */ - @Override - public String toString() { - return "MultiKey" + Arrays.toString(keys); + protected Object readResolve() { + calculateHashCode(keys); + return this; } /** - * Calculate the hash code of the instance using the provided keys. - * @param keys the keys to calculate the hash code for + * Gets the size of the list of keys. + * + * @return the size of the list of keys + * @since 3.1 */ - private void calculateHashCode(final Object[] keys) - { - int total = 0; - for (final Object key : keys) { - if (key != null) { - total ^= key.hashCode(); - } - } - hashCode = total; + public int size() { + return keys.length; } /** - * Recalculate the hash code after deserialization. The hash code of some - * keys might have change (hash codes based on the system hash code are - * only stable for the same process). - * @return the instance with recalculated hash code + * Gets a debugging string version of the key. + * + * @return a debugging string */ - protected Object readResolve() { - calculateHashCode(keys); - return this; + @Override + public String toString() { + return "MultiKey" + Arrays.toString(keys); } } diff --git a/src/test/java/org/apache/commons/collections4/keyvalue/MultiKeyTest.java b/src/test/java/org/apache/commons/collections4/keyvalue/MultiKeyTest.java index f7e224d..2ba04b6 100644 --- a/src/test/java/org/apache/commons/collections4/keyvalue/MultiKeyTest.java +++ b/src/test/java/org/apache/commons/collections4/keyvalue/MultiKeyTest.java @@ -53,6 +53,7 @@ public class MultiKeyTest { } } + static class SystemHashCodeSimulatingKey implements Serializable { private static final long serialVersionUID = -1736147315703444603L; @@ -82,13 +83,12 @@ public class MultiKeyTest { return this; } } - Integer ONE = Integer.valueOf(1); + Integer TWO = Integer.valueOf(2); Integer THREE = Integer.valueOf(3); Integer FOUR = Integer.valueOf(4); Integer FIVE = Integer.valueOf(5); - //----------------------------------------------------------------------- @Test public void testConstructors() throws Exception { @@ -294,4 +294,12 @@ public class MultiKeyTest { assertEquals(7, new MultiKey<>(new Integer[] { ONE, TWO, ONE, TWO, ONE, TWO, ONE }).size()); } + @Test + public void testTwoArgCtor() { + MultiKeyTest key1 = new MultiKeyTest(); + MultiKeyTest key2 = new MultiKeyTest(); + MultiKeyTest[] keys = new MultiKey<>(key1, key2).getKeys(); + assertNotNull(keys); + } + }