sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1683327 - in /sis/branches/JDK8/core/sis-referencing/src: main/java/org/apache/sis/parameter/Parameters.java test/java/org/apache/sis/parameter/ParametersTest.java
Date Wed, 03 Jun 2015 13:37:34 GMT
Author: desruisseaux
Date: Wed Jun  3 13:37:33 2015
New Revision: 1683327

URL: http://svn.apache.org/r1683327
Log:
Referencing: fix a Parameters.copy(...) bug, which was not copying correctly the subgroups.
https://issues.apache.org/jira/browse/SIS-202

Modified:
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/parameter/Parameters.java
    sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/parameter/ParametersTest.java

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/parameter/Parameters.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/parameter/Parameters.java?rev=1683327&r1=1683326&r2=1683327&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/parameter/Parameters.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/parameter/Parameters.java
[UTF-8] Wed Jun  3 13:37:33 2015
@@ -646,24 +646,19 @@ public abstract class Parameters impleme
     public static void copy(final ParameterValueGroup values, final ParameterValueGroup destination)
             throws InvalidParameterNameException, InvalidParameterValueException
     {
-        final Integer ONE = 1;
+        final Integer ZERO = 0;
         final Map<String,Integer> occurrences = new HashMap<>();
         for (final GeneralParameterValue value : values.values()) {
             final String name = value.getDescriptor().getName().getCode();
+            final int occurrence = occurrences.getOrDefault(name, ZERO);
             if (value instanceof ParameterValueGroup) {
                 /*
                  * Contains sub-group - invokes 'copy' recursively.
+                 * The target group may exist, but not necessarily.
                  */
-                final GeneralParameterDescriptor descriptor;
-                descriptor = destination.getDescriptor().descriptor(name);
-                if (descriptor instanceof ParameterDescriptorGroup) {
-                    final ParameterValueGroup groups = (ParameterValueGroup) descriptor.createValue();
-                    copy((ParameterValueGroup) value, groups);
-                    values.groups(name).add(groups);
-                } else {
-                    throw new InvalidParameterNameException(Errors.format(
-                            Errors.Keys.UnexpectedParameter_1, name), name);
-                }
+                final List<ParameterValueGroup> groups = destination.groups(name);
+                copy((ParameterValueGroup) value, (occurrence < groups.size())
+                        ? groups.get(occurrence) : destination.addGroup(name));
             } else {
                 /*
                  * Single parameter - copy the value, with special care for value with units
@@ -672,9 +667,7 @@ public abstract class Parameters impleme
                  */
                 final ParameterValue<?> source = (ParameterValue<?>) value;
                 final ParameterValue<?> target;
-                Integer occurrence = occurrences.get(name);
-                if (occurrence == null) {
-                    occurrence = ONE;
+                if (occurrence == 0) {
                     try {
                         target = destination.parameter(name);
                     } catch (ParameterNotFoundException cause) {
@@ -683,9 +676,7 @@ public abstract class Parameters impleme
                     }
                 } else {
                     target = (ParameterValue<?>) getOrCreate(destination, name, occurrence);
-                    occurrence++;
                 }
-                occurrences.put(name, occurrence);
                 final Object  v    = source.getValue();
                 final Unit<?> unit = source.getUnit();
                 if (unit == null) {
@@ -699,6 +690,7 @@ public abstract class Parameters impleme
                             Errors.Keys.IllegalArgumentValue_2, name, v), name, v);
                 }
             }
+            occurrences.put(name, occurrence + 1);
         }
     }
 

Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/parameter/ParametersTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/parameter/ParametersTest.java?rev=1683327&r1=1683326&r2=1683327&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/parameter/ParametersTest.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/parameter/ParametersTest.java
[UTF-8] Wed Jun  3 13:37:33 2015
@@ -20,6 +20,7 @@ import java.util.Set;
 import java.util.Collection;
 import java.util.Collections;
 import javax.measure.unit.SI;
+import javax.measure.unit.Unit;
 import org.opengis.parameter.ParameterDescriptor;
 import org.opengis.parameter.ParameterDirection;
 import org.opengis.parameter.ParameterValue;
@@ -27,12 +28,12 @@ import org.opengis.parameter.ParameterVa
 import org.opengis.metadata.Identifier;
 import org.opengis.util.GenericName;
 import org.opengis.util.InternationalString;
-import javax.measure.unit.Unit;
 import org.apache.sis.measure.Range;
 import org.apache.sis.measure.NumberRange;
 import org.apache.sis.measure.MeasurementRange;
 import org.apache.sis.test.DependsOn;
 import org.apache.sis.test.TestCase;
+import org.apache.sis.test.TestUtilities;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -135,28 +136,55 @@ public final strictfp class ParametersTe
 
     /**
      * Tests {@link Parameters#copy(ParameterValueGroup, ParameterValueGroup)}.
+     *
+     * @see <a href="https://issues.apache.org/jira/browse/SIS-202">SIS-202</a>
      */
     @Test
     public void testCopy() {
-        final ParameterValueGroup source = DefaultParameterDescriptorGroupTest.M1_M1_O1_O2.createValue();
-        final ParameterValue<?> o1 = source.parameter("Optional 4");
+        /*
+         * The descriptor to be used for this test. This descriptor contain at least
+         * one subgroup, for testing the Parameters.copy(...) method recursivity.
+         */
+        final String subgroupName = DefaultParameterDescriptorGroupTest.M1_M1_O1_O2.getName().getCode();
+        final DefaultParameterDescriptorGroup descriptor = new DefaultParameterDescriptorGroup(
+                Collections.singletonMap(DefaultParameterDescriptorGroup.NAME_KEY, "parent"),
1, 1,
+                DefaultParameterDescriptorTest.createSimpleOptional("A parent parameter",
String.class),
+                DefaultParameterDescriptorGroupTest.M1_M1_O1_O2);
+        /*
+         * Create the parameter value to copy. We set some values, but intentionally not
all of them.
+         * The unset values will be used for verifying that they do not overwrite destination
values.
+         */
+        final ParameterValueGroup source = descriptor.createValue();
+        final ParameterValueGroup sourceSubgroup = source.addGroup(subgroupName);
+        final ParameterValue<?> o1 = sourceSubgroup.parameter("Optional 4");
         final ParameterValue<?> o2 = o1.getDescriptor().createValue(); // See ParameterFormatTest.testMultiOccurrence()
-        source.parameter("Mandatory 2").setValue(20);
-        source.values().add(o2);
+        sourceSubgroup.parameter("Mandatory 2").setValue(20);
+        sourceSubgroup.values().add(o2);
         o1.setValue(40);
         o2.setValue(50);
-
-        final ParameterValueGroup destination = DefaultParameterDescriptorGroupTest.M1_M1_O1_O2.createValue();
-        destination.parameter("Mandatory 1").setValue(-10);  // We expect this value to be
overwritten.
-        destination.parameter("Optional 3") .setValue( 30);  // We expect this value to be
preserved.
-        Parameters.copy(source, destination);
-
-        assertEquals("Mandatory 1", 10, destination.parameter("Mandatory 1").intValue());
-        assertEquals("Mandatory 2", 20, destination.parameter("Mandatory 2").intValue());
-        assertEquals("Optional 3",  30, destination.parameter("Optional 3") .intValue());
-        assertEquals("Optional 4",  40, destination.parameter("Optional 4") .intValue());
+        source.parameter("A parent parameter").setValue("A value from the source");
+        /*
+         * Create the parameter to use as the destination. We put some value in those parameters
in order to
+         * verify that those values are overwritten (only those for which the value is set
in the source).
+         */
+        final ParameterValueGroup target = descriptor.createValue();
+        final ParameterValueGroup targetSubgroup = target.addGroup(subgroupName);
+        targetSubgroup.parameter("Mandatory 1").setValue(-10);  // We expect this value to
be overwritten.
+        targetSubgroup.parameter("Optional 3") .setValue( 30);  // We expect this value to
be preserved.
+        target.parameter("A parent parameter") .setValue("A value to be overwritten");
+        /*
+         * The actual test.
+         */
+        Parameters.copy(source, target);
+        assertSame(sourceSubgroup, TestUtilities.getSingleton(source.groups(subgroupName)));
+        assertSame(targetSubgroup, TestUtilities.getSingleton(target.groups(subgroupName)));
+        assertEquals("A value from the source", target.parameter("A parent parameter").getValue());
+        assertEquals("Mandatory 1", 10, targetSubgroup.parameter("Mandatory 1").intValue());
+        assertEquals("Mandatory 2", 20, targetSubgroup.parameter("Mandatory 2").intValue());
+        assertEquals("Optional 3",  30, targetSubgroup.parameter("Optional 3") .intValue());
+        assertEquals("Optional 4",  40, targetSubgroup.parameter("Optional 4") .intValue());
         assertEquals("Optional 4 (second occurrence)", 50,
-                ((ParameterValue<?>) destination.values().get(4)).intValue());
+                ((ParameterValue<?>) targetSubgroup.values().get(4)).intValue());
     }
 
     /**



Mime
View raw message