ICU-10017 Correctly handle all types of Number fields in TimePeriod.

X-SVN-Rev: 33479

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/util/TimePeriod.java b/icu4j/main/classes/core/src/com/ibm/icu/util/TimePeriod.java
index 55b8689a76d6d48e1c0d77b2cc056d881f4b0260..1ed5ff06ca33a76a0eba6a012eaf38ee6702ddc4 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/util/TimePeriod.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/util/TimePeriod.java
@@ -6,10 +6,15 @@
  */
 package com.ibm.icu.util;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.math.BigInteger;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 
+import com.ibm.icu.math.BigDecimal;
+
 /**
  * TimePeriod represents a time period.  TimePeriod objects are immutable.
  * <p>Example usage:
@@ -37,7 +42,10 @@ public final class TimePeriod implements Iterable<TimeUnitAmount> {
     /**
      * Returns a new TimePeriod that matches the given time unit amounts.
      * @param amounts the TimeUnitAmounts. Must be non-empty. Normalization of the
-     *   amounts and inclusion/exclusion of 0 amounts is up to caller.
+     *   amounts and inclusion/exclusion of 0 amounts is up to caller. The Number
+     *   in each TimeUnitAmount must either be a Byte, Short, Integer, Long, Float,
+     *   Double, BigInteger, or BigDecimal or it must implement Cloneable and have
+     *   a public clone method.
      * @return the new TimePeriod object
      * @throws IllegalArgumentException if multiple TimeUnitAmount objects match
      * the same time unit or if any but the smallest TimeUnit has a fractional value
@@ -51,7 +59,10 @@ public final class TimePeriod implements Iterable<TimeUnitAmount> {
     /**
      * Returns a new TimePeriod that matches the given time unit amounts.
      * @param amounts the TimeUnitAmounts. Must be non-empty. Normalization of the
-     *   amounts and inclusion/exclusion of 0 amounts is up to caller.
+     *   amounts and inclusion/exclusion of 0 amounts is up to caller. The Number
+     *   in each TimeUnitAmount must either be a Byte, Short, Integer, Long, Float,
+     *   Double, BigInteger, or BigDecimal or it must implement Cloneable and have
+     *   a public clone method.
      * @return the new TimePeriod object
      * @throws IllegalArgumentException if multiple TimeUnitAmount objects match
      * the same time unit or if any but the smallest TimeUnit has a fractional value
@@ -70,7 +81,7 @@ public final class TimePeriod implements Iterable<TimeUnitAmount> {
             // This line is necessary to guarantee immutability of the TimePeriod
             // class. A Number object, which is in TimeUnitAmount, need not be immutable,
             // but Double is immutable.
-            fields[index] = new TimeUnitAmount(tua.getNumber().doubleValue(), tua.getTimeUnit());
+            fields[index] = cloneIfNecessary(tua);
             size++;
         }
         if (size == 0) {
@@ -100,7 +111,7 @@ public final class TimePeriod implements Iterable<TimeUnitAmount> {
      * @draft ICU 52
      */
     public TimeUnitAmount getAmount(TimeUnit timeUnit) {
-        return fields[timeUnit.getIndex()];
+        return cloneIfNecessary(fields[timeUnit.getIndex()]);
     }
 
     /**
@@ -155,6 +166,43 @@ public final class TimePeriod implements Iterable<TimeUnitAmount> {
         return result;
     }
     
+    private static TimeUnitAmount cloneIfNecessary(TimeUnitAmount tua) {
+        if (tua == null) {
+            return null;
+        }
+        Number number = tua.getNumber();
+        if (number instanceof Double || number instanceof Integer ||
+            number instanceof Long || number instanceof Float ||
+            number instanceof Byte || number instanceof Short ||
+            number instanceof BigInteger || number instanceof BigDecimal) {
+          return tua;            
+        }
+        return new TimeUnitAmount(cloneNumber(number), tua.getTimeUnit());
+    }
+    
+    private static Number cloneNumber(Number number) {
+        // Since Number doesn't implement Cloneable, we have to use reflection
+        // to find the clone() method. If this method throws an exception, it
+        // means that the subclass of Number can't be cloned.
+        Class<? extends Number> clz = number.getClass();
+        Method cloneMethod;
+        try {
+            cloneMethod = clz.getMethod("clone");
+            cloneMethod.setAccessible(true);
+            return (Number) cloneMethod.invoke(number);
+        } catch (NoSuchMethodException e) {
+            throw new RuntimeException(e);
+        } catch (SecurityException e) {
+            throw new RuntimeException(e);
+        } catch (IllegalAccessException e) {
+            throw new RuntimeException(e);
+        } catch (IllegalArgumentException e) {
+            throw new RuntimeException(e);
+        } catch (InvocationTargetException e) {
+            throw new RuntimeException(e);
+        }
+    }
+    
     private class TPIterator implements Iterator<TimeUnitAmount> {
         
         private int index = 0;
@@ -176,6 +224,6 @@ public final class TimePeriod implements Iterable<TimeUnitAmount> {
         public void remove() {
             throw new UnsupportedOperationException();           
         }
-    }
+    } 
 }
 

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeUnitTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeUnitTest.java
index 090b01a6ee16917312a31f0317260cd1c38fb9ed..d9b4d9f1b54fe4cbb332ad46dc7fc28f7ff8b455 100644 (file)
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeUnitTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeUnitTest.java
@@ -398,6 +398,23 @@ public class TimeUnitTest extends TestFmwk {
         }
     }
     
+    public void TestTimePeriodWithMutableNumber() {
+        MutableInt mutableInput = new MutableInt(3);
+        TimePeriod tp = TimePeriod.forAmounts(
+                new TimeUnitAmount(mutableInput, TimeUnit.HOUR));
+        mutableInput.set(5);
+        MutableInt mutableOutput = (MutableInt) tp.getAmount(TimeUnit.HOUR).getNumber();
+        assertEquals(
+                "Mutating input shouldn't affect TimePeriod.",
+                3,
+                mutableOutput.intValue());
+        mutableOutput.set(5);
+        assertEquals(
+                "Mutating output shouldn't affect TimePeriod.",
+                3,
+                ((MutableInt) tp.getAmount(TimeUnit.HOUR).getNumber()).intValue());       
+    }
+    
     private void verifyFormatPeriod(String desc, TimeUnitFormat tuf, Object[][] testData) {
         StringBuilder builder = new StringBuilder();
         boolean failure = false;
@@ -412,4 +429,46 @@ public class TimeUnitTest extends TestFmwk {
             errln(builder.toString());
         }
     }
+    
+    private static class MutableInt extends Number implements Cloneable {
+
+        private int value;
+        
+        public MutableInt(int x) {
+            value = x;
+        }
+        
+        @Override
+        public int intValue() {
+            return value;
+        }
+
+        @Override
+        public long longValue() {
+            return value;
+        }
+
+        @Override
+        public float floatValue() {
+            return value;
+        }
+
+        @Override
+        public double doubleValue() {
+            return value;
+        }
+        
+        public void set(int x) {
+            value = x;
+        }
+        
+        @Override
+        public MutableInt clone() {
+            try {
+                return (MutableInt) super.clone();
+            } catch (CloneNotSupportedException e) {
+                return null;
+            }
+        }
+    }
 }