sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 01/01: Fix a CorruptedObjectException when registering two aliases for the same unit and fix wrong unit symbol on ConventionalUnit resulting from an operation.
Date Wed, 19 Sep 2018 14:52:16 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git

commit 0b1764266166c86f16121018ee4e41383b1e7cb6
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Wed Sep 19 16:46:36 2018 +0200

    Fix a CorruptedObjectException when registering two aliases for the same unit
    and fix wrong unit symbol on ConventionalUnit resulting from an operation.
---
 .../org/apache/sis/measure/ConventionalUnit.java   |  2 +-
 .../java/org/apache/sis/measure/SystemUnit.java    | 98 +++++++++++++++++-----
 .../java/org/apache/sis/measure/UnitFormat.java    |  4 +-
 .../main/java/org/apache/sis/measure/Units.java    |  2 +-
 .../org/apache/sis/measure/UnitFormatTest.java     | 21 +++++
 5 files changed, 102 insertions(+), 25 deletions(-)

diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java b/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
index ef66a06..a0f3e9f 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
@@ -357,7 +357,7 @@ final class ConventionalUnit<Q extends Quantity<Q>> extends
AbstractUnit<Q> {
 
     /**
      * Returns a new unit identical to this unit except for the symbol, which is set to the
given value.
-     * This is used by {@link UnitFormat} only; we do not provide public API for setting
a unit symbol
+     * This is used by {@link UnitFormat} mostly; we do not provide public API for setting
a unit symbol
      * on a conventional unit.
      */
     final ConventionalUnit<Q> forSymbol(final String symbol) {
diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java b/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
index 6d51b8e..d46cdf6 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
@@ -57,6 +57,17 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
     private static final long serialVersionUID = 4097466138698631914L;
 
     /**
+     * The non-empty symbol for {@link Units#UNITY}.
+     */
+    static final String ONE = "1";
+
+    /**
+     * Maximum number of multiplications or divisions in the symbol of the first unit
+     * for allowing the use of that symbol for inferring a new symbol.
+     */
+    private static final int MAX_OPERATIONS_IN_SYMBOL = 1;
+
+    /**
      * The type of quantity that uses this unit, or {@code null} if unknown.
      */
     final Class<Q> quantity;
@@ -110,40 +121,52 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
      *
      * @param  operation  symbol to write after the symbol of this unit for generating the
new unit symbol, or 0
      *                    for not inferring new symbol. Ignored if the condition documented
in javadoc does not hold.
-     * @param  other      other units to append after the operation symbol, or {@code null}
if none.
+     * @param  other      other units to append after the operation symbol, or {@code null}
if none or should be ignored.
      *                    Ignored if the condition documented in javadoc does not hold.
      */
     private SystemUnit<?> create(final UnitDimension newDimension, final char operation,
final Unit<?> other) {
         /*
+         * Check if we are computing the inverse of the other unit. We are computing inverse
+         * if this unit is unity ("1") and is used as the numerator of a division. The unity
+         * symbol is an empty string but we will format it as "1".
+         */
+        String ts = getSymbol();
+        final boolean inverse = (ts != null && ts.isEmpty() && operation
== DIVIDE);
+        /*
          * Check if the SystemUnit to create is known to Units, provided that no dimensionless
units
          * is involved. If a dimensionless unit is involved, we will try to build a symbol
before to
          * check if the unit is known to Units. The reason is that there is many dimensionless
units
          * with different symbols (rad, sr, psu, …).
          */
-        final boolean deferred = newDimension.isDimensionless() || dimension.isDimensionless()
||
+        final boolean deferred = newDimension.isDimensionless() || (!inverse && dimension.isDimensionless())
||
                 (other != null && UnitDimension.isDimensionless(other.getDimension()));
         if (!deferred) {
             final SystemUnit<?> result = Units.get(newDimension);
             if (result != null) return result;
         }
+        /*
+         * Try to create a unit symbol as the concatenation of the symbols of the two units,
+         * with the operation symbol between them. If we can not, 'symbol' will stay null.
+         */
         String symbol = null;
         if (operation != 0) {
-            final String ts = getSymbol();
-            final boolean exponents = (operation == MULTIPLY || operation == DIVIDE);
-            if (invalidCharForSymbol(ts, exponents ? 1 : 0, exponents) == -1) {
-                if (other == null) {
-                    symbol = (ts + operation).intern();
-                } else {
+            final boolean allowExponents = (operation == MULTIPLY || operation == DIVIDE);
+            if (inverse || isValidSymbol(ts, allowExponents ? MAX_OPERATIONS_IN_SYMBOL :
0, allowExponents)) {
+                if (other != null) {
                     final String os = other.getSymbol();
-                    if (invalidCharForSymbol(os, 0, exponents) == -1) {
+                    if (isValidSymbol(os, 0, allowExponents)) {
+                        if (inverse) ts = ONE;
                         symbol = (ts + operation + os).intern();
                     }
+                } else if (!allowExponents) {
+                    symbol = (ts + operation).intern();             // The operation is an
exponent.
                 }
             }
         }
         /*
-         * The check that we did not performed at the beginning of this method
-         * if any component were unitless.
+         * Performs now the check that we did not performed at the
+         * beginning of this method if any component was unitless.
+         * The difference is that we take unit symbol in account.
          */
         if (deferred) {
             final SystemUnit<?> result = Units.get(newDimension);
@@ -165,6 +188,18 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
     }
 
     /**
+     * Returns {@code true} if the given unit symbol is valid.
+     *
+     * @param  symbol          the symbol to verify for validity.
+     * @param  maxMultiply     maximal number of multiplication symbol to accept.
+     * @param  allowExponents  whether to accept also exponent characters.
+     * @return {@code true} if the given symbol is valid.
+     */
+    private static boolean isValidSymbol(final String symbol, int maxMultiply, final boolean
allowExponents) {
+        return invalidCharForSymbol(symbol, maxMultiply, allowExponents) == -1;
+    }
+
+    /**
      * If the given symbol contains an invalid character for a unit symbol, returns the character
code point.
      * Otherwise if the given symbol is null or empty, returns -2. Otherwise (the symbol
is valid) returns -1.
      *
@@ -175,11 +210,12 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
      * {@link UnitFormat} to create a new one from the base units. The criterion for accepting
a symbol or not (for
      * example how many multiplications) is arbitrary.</p>
      *
-     * @param  symbol     the symbol to verify for invalid characters.
-     * @param  multiply   maximal number of multiplication symbol to accept.
-     * @param  exponents  whether to accept also exponent characters.
+     * @param  symbol          the symbol to verify for invalid characters.
+     * @param  maxMultiply     maximal number of multiplication symbol to accept.
+     * @param  allowExponents  whether to accept also exponent characters.
+     * @return Unicode code point of the invalid character, or a negative value.
      */
-    private static int invalidCharForSymbol(final String symbol, int multiply, final boolean
exponents) {
+    private static int invalidCharForSymbol(final String symbol, int maxMultiply, final boolean
allowExponents) {
         if (symbol == null || symbol.isEmpty()) {
             return -2;
         }
@@ -187,8 +223,8 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
             final int c = symbol.codePointAt(i);
             if (!isSymbolChar(c)) {
                 if (c == MULTIPLY) {
-                    if (--multiply < 0) return c;
-                } else if (!exponents || !Characters.isSuperScript(c)) {
+                    if (--maxMultiply < 0) return c;
+                } else if (!allowExponents || !Characters.isSuperScript(c)) {
                     return c;
                 }
             }
@@ -505,7 +541,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
      *
      * @param  inverse  wether to use the inverse of {@code other}.
      */
-    private <T extends Quantity<T>> Unit<?> product(final Unit<T>
other, final boolean inverse) {
+    private <T extends Quantity<T>> Unit<?> product(final Unit<T>
other, boolean inverse) {
         final Unit<T> intermediate = other.getSystemUnit();
         final Dimension dim = intermediate.getDimension();
         final UnitDimension newDimension;
@@ -517,14 +553,32 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q>
implements
             operation = MULTIPLY;
             newDimension = dimension.multiply(dim);
         }
-        Unit<?> result = create(newDimension, operation, other);
-        if (intermediate != other) {
+        final boolean transformed = (intermediate != other);
+        Unit<?> result = create(newDimension, operation, transformed ? null : other);
+        if (transformed) {
             UnitConverter c = other.getConverterTo(intermediate);
             if (!c.isLinear()) {
                 throw new IllegalArgumentException(Errors.format(Errors.Keys.NonRatioUnit_1,
other));
             }
-            if (inverse) c = c.inverse();
-            result = result.transform(c);
+            if (!c.isIdentity()) {
+                if (inverse) c = c.inverse();
+                result = result.transform(c);
+                /*
+                 * If the system unit product is an Apache SIS implementation, try to infer
a unit symbol
+                 * to be given to our customized 'transform' method. Otherwise fallback on
standard API.
+                 */
+                if (result.getSymbol() == null && result instanceof ConventionalUnit<?>)
{
+                    String ts = getSymbol();
+                    inverse &= (ts != null && ts.isEmpty());
+                    if (inverse || isValidSymbol(ts, MAX_OPERATIONS_IN_SYMBOL, true)) {
+                        final String os = other.getSymbol();
+                        if (isValidSymbol(os, 0, true)) {
+                            if (inverse) ts = ONE;
+                            result = ((ConventionalUnit<?>) result).forSymbol((ts +
operation + os).intern());
+                        }
+                    }
+                }
+            }
         }
         return result;
     }
diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/UnitFormat.java b/core/sis-utility/src/main/java/org/apache/sis/measure/UnitFormat.java
index 42e808a..fb18ac4 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/measure/UnitFormat.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/measure/UnitFormat.java
@@ -427,10 +427,12 @@ public class UnitFormat extends Format implements javax.measure.format.UnitForma
              */
             throw new CorruptedObjectException("unitToLabel");
         }
-        if (unitForOldLabel != null && !unitForOldLabel.equals(unit)) {
+        if (unitForOldLabel != null && !unitForOldLabel.getSystemUnit().equals(unit.getSystemUnit()))
{
             /*
              * Assuming there is no bug in our algorithm, this exception should never happen
              * unless this UnitFormat has been modified concurrently in another thread.
+             * We compared system units because the units may not be strictly equal
+             * as a result of the call to ConventionalUnit.forSymbol(label).
              */
             throw new CorruptedObjectException("labelToUnit");
         }
diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java b/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java
index 3d402f9..48ceabd 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java
@@ -1238,7 +1238,7 @@ public final class Units extends Static {
         UnitRegistry.alias(HECTARE,   "hm²");
         UnitRegistry.alias(LITRE,       "l");
         UnitRegistry.alias(LITRE,       "ℓ");
-        UnitRegistry.alias(UNITY,       "1");
+        UnitRegistry.alias(UNITY, SystemUnit.ONE);
 
         initialized = true;
     }
diff --git a/core/sis-utility/src/test/java/org/apache/sis/measure/UnitFormatTest.java b/core/sis-utility/src/test/java/org/apache/sis/measure/UnitFormatTest.java
index 6ba7095..ee3073c 100644
--- a/core/sis-utility/src/test/java/org/apache/sis/measure/UnitFormatTest.java
+++ b/core/sis-utility/src/test/java/org/apache/sis/measure/UnitFormatTest.java
@@ -206,6 +206,17 @@ public final strictfp class UnitFormatTest extends TestCase {
     }
 
     /**
+     * Tests the assignation of two labels on the same unit.
+     */
+    @Test
+    public void testDuplicatedLabels() {
+        final UnitFormat f = new UnitFormat(Locale.ENGLISH);
+        f.label(Units.DEGREE, "deg");
+        f.label(Units.DEGREE, "dd");        // For "decimal degrees"
+        roundtrip(f, "dd", "dd");
+    }
+
+    /**
      * Tests unit formatting with {@link UnitFormat.Style#UCUM}.
      */
     @Test
@@ -510,6 +521,16 @@ public final strictfp class UnitFormatTest extends TestCase {
     }
 
     /**
+     * Tests the parsing of {@code "1/l"}.
+     */
+    @Test
+    public void testParseInverseL() {
+        final UnitFormat f = new UnitFormat(Locale.UK);
+        final Unit<?> u = f.parse("1/l");
+        assertEquals("1∕L", u.toString());
+    }
+
+    /**
      * Tests parsing of symbols containing an explicit exponentiation operation.
      * Usually the exponentiation is implicit, as in {@code "m*s-1"}.
      * However some formats write it explicitly, as in {@code "m*s^-1"}.


Mime
View raw message