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
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 31a6086 Add tests and fix missed sharing of WKT values.
31a6086 is described below
commit 31a6086b2f8c97dc8143791b5326dc68eac38faa
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Wed Nov 18 17:02:07 2020 +0100
Add tests and fix missed sharing of WKT values.
---
.../java/org/apache/sis/io/wkt/StoredTree.java | 57 +++++++++++---
.../java/org/apache/sis/io/wkt/WKTDictionary.java | 77 ++++++++++++++++--
.../org/apache/sis/io/wkt/WKTDictionaryTest.java | 91 +++++++++++++++++++++-
3 files changed, 204 insertions(+), 21 deletions(-)
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/StoredTree.java
b/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/StoredTree.java
index 5800f5b..d29c13c 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/StoredTree.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/StoredTree.java
@@ -21,6 +21,7 @@ import java.util.LinkedList;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Stream;
+import java.util.function.Consumer;
import java.io.Serializable;
import org.apache.sis.util.ArraysExt;
import org.apache.sis.internal.referencing.WKTKeywords;
@@ -90,14 +91,15 @@ final class StoredTree implements Serializable {
* separated array for making possible to share {@code Node} instances.
*/
Node(final Deflater deflater, final Element element) {
- keyword = element.keyword;
+ keyword = (String) deflater.unique(element.keyword);
children = element.getChildren();
if (children != null) {
for (int i=0; i<children.length; i++) {
- final Object child = children[i];
+ Object child = children[i];
if (child instanceof Element) {
- children[i] = deflater.unique(new Node(deflater,
(Element) child));
+ child = new Node(deflater, (Element) child);
}
+ children[i] = deflater.unique(child);
}
}
deflater.addOffset(element);
@@ -137,9 +139,9 @@ final class StoredTree implements Serializable {
final Node peekLastElement(final String... keys) {
if (children != null) {
for (int i = children.length; --i >= 0;) {
- final Object value = children[i];
- if (value instanceof Node) {
- final Node node = (Node) value;
+ final Object object = children[i];
+ if (object instanceof Node) {
+ final Node node = (Node) object;
if (node.children != null) {
for (final String key : keys) {
if (node.keyword.equalsIgnoreCase(key)) {
@@ -171,6 +173,25 @@ final class StoredTree implements Serializable {
}
/**
+ * Adds keyword and children to the given supplier.
+ * Children of children are added recursively.
+ * This method is for testing purposes only.
+ *
+ * @see StoredTree#forEachValue(Consumer)
+ */
+ final void forEachValue(final Consumer<Object> addTo) {
+ addTo.accept(keyword);
+ if (children != null) {
+ for (final Object child : children) {
+ addTo.accept(child);
+ if (child instanceof Node) {
+ ((Node) child).forEachValue(addTo);
+ }
+ }
+ }
+ }
+
+ /**
* Returns the string representation of the first value, which is
usually the element name.
* For example in {@code DATUM["WGS 84", …]} this is "WGS 84". If
there is no children then
* this method returns the keyword, which is usually an enumeration
value (for example "NORTH"}).
@@ -259,7 +280,7 @@ final class StoredTree implements Serializable {
*/
StoredTree(final Element root, final Map<Object,Object> sharedValues) {
final Deflater deflater = new Deflater(sharedValues);
- this.root = deflater.unique(new Node(deflater, root));
+ this.root = (Node) deflater.unique(new Node(deflater, root));
offsets = deflater.offsets();
}
@@ -322,9 +343,9 @@ final class StoredTree implements Serializable {
* @see Node#hashCode()
* @see Node#equals(Object)
*/
- final Node unique(final Node node) {
+ final Object unique(final Object node) {
final Object existing = sharedValues.putIfAbsent(node, node);
- return (existing != null) ? (Node) existing : node;
+ return (existing != null) ? existing : node;
}
/**
@@ -336,7 +357,7 @@ final class StoredTree implements Serializable {
if (count >= offsets.length) {
offsets = Arrays.copyOf(offsets, count * 2);
}
- int offset = Math.min(Short.MAX_VALUE, element.offset);
+ int offset = Math.min(Short.MAX_VALUE, Math.max(0,
element.offset));
if (element.isFragment) offset = ~offset;
offsets[count++] = (short) offset;
}
@@ -348,10 +369,10 @@ final class StoredTree implements Serializable {
@SuppressWarnings("ReturnOfCollectionOrArrayField")
final short[] offsets() {
offsets = ArraysExt.resize(offsets, count);
- final Deflater other = (Deflater) sharedValues.putIfAbsent(this,
this);
+ final short[] other = (short[]) sharedValues.putIfAbsent(this,
offsets);
sharedValues = null;
if (other != null) {
- this.offsets = other.offsets;
+ offsets = other;
}
return offsets;
}
@@ -363,14 +384,17 @@ final class StoredTree implements Serializable {
*/
@Override
public boolean equals(final Object other) {
+ assert offsets.length == count;
return (other instanceof Deflater) && Arrays.equals(offsets,
((Deflater) other).offsets);
}
/**
* Computes a hash code value based only on the {@link #offsets} array.
+ * See {@link #equals(Object)} for rational.
*/
@Override
public int hashCode() {
+ assert offsets.length == count;
return Arrays.hashCode(offsets);
}
}
@@ -465,6 +489,15 @@ final class StoredTree implements Serializable {
}
/**
+ * Adds keywords and children to the given supplier. This is for testing
purposes only.
+ *
+ * @see WKTDictionary#forEachValue(Consumer)
+ */
+ final void forEachValue(final Consumer<Object> addTo) {
+ root.forEachValue(addTo);
+ }
+
+ /**
* Returns the keyword of the root element.
*/
final String keyword() {
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/WKTDictionary.java
b/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/WKTDictionary.java
index a20ff85..7548254 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/WKTDictionary.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/io/wkt/WKTDictionary.java
@@ -32,7 +32,9 @@ import java.io.BufferedReader;
import java.io.IOException;
import java.text.ParseException;
import java.text.ParsePosition;
+import java.util.function.Consumer;
import org.opengis.util.FactoryException;
+import org.opengis.util.InternationalString;
import org.opengis.metadata.Identifier;
import org.opengis.metadata.citation.Citation;
import org.opengis.referencing.IdentifiedObject;
@@ -53,6 +55,7 @@ import org.apache.sis.util.ArraysExt;
import org.apache.sis.util.Exceptions;
import org.apache.sis.util.resources.Errors;
import org.apache.sis.util.collection.FrequencySortedSet;
+import org.apache.sis.util.iso.SimpleInternationalString;
/**
@@ -99,7 +102,7 @@ public class WKTDictionary extends GeodeticAuthorityFactory {
* @see #updateAuthority()
* @see #getAuthority()
*/
- private Citation authority;
+ private volatile Citation authority;
/**
* Authorities declared in all {@code "ID[CITATION[…]]"} elements found in
WKT definitions.
@@ -714,6 +717,26 @@ public class WKTDictionary extends
GeodeticAuthorityFactory {
}
/**
+ * Adds all definition values to the given supplier. This is for testing
purposes only.
+ * This method performs no locking because it is not needed for current
JUnit tests.
+ *
+ * @see StoredTree#forEachValue(Consumer)
+ */
+ final void forEachValue(final Consumer<Object> addTo) {
+ for (final Object value : definitions.values()) {
+ if (value instanceof Disambiguation) {
+ Disambiguation choices = (Disambiguation) value;
+ do {
+ addTo.accept(choices.value);
+ choices = choices.previous;
+ } while (choices != null);
+ } else {
+ addTo.accept(value);
+ }
+ }
+ }
+
+ /**
* Returns the authority or specification that defines the codes
recognized by this factory.
* This is the first of the following values, in preference order:
*
@@ -748,6 +771,30 @@ public class WKTDictionary extends
GeodeticAuthorityFactory {
}
/**
+ * Gets a description of the object corresponding to a code.
+ *
+ * @param code value allocated by authority.
+ * @return a description of the object, or {@code null} if {@code null} if
none.
+ * @throws NoSuchAuthorityCodeException if the specified {@code code} was
not found.
+ * @throws FactoryException if the query failed for some other reason.
+ */
+ @Override
+ public InternationalString getDescriptionText(final String code) throws
FactoryException {
+ final String text;
+ final Object value = getOrCreate(code, false);
+ if (value instanceof IdentifiedObject) {
+ text = ((IdentifiedObject) value).getName().getCode();
+ } else {
+ text = String.valueOf(value);
+ if (!(value instanceof StoredTree)) {
+ // Exception message saved in a previous invocation of
`getOrCreate(…)`.
+ throw new FactoryException(text);
+ }
+ }
+ return new SimpleInternationalString(text);
+ }
+
+ /**
* Returns an arbitrary object from a code.
*
* @param code value allocated by authority.
@@ -757,6 +804,25 @@ public class WKTDictionary extends
GeodeticAuthorityFactory {
*/
@Override
public IdentifiedObject createObject(final String code) throws
FactoryException {
+ final Object value = getOrCreate(code, true);
+ if (value instanceof IdentifiedObject) {
+ return (IdentifiedObject) value;
+ } else {
+ // Exception message saved in a previous invocation of
`getOrCreate(…)`.
+ throw new FactoryException(String.valueOf(value));
+ }
+ }
+
+ /**
+ * Returns the object associated to the given code.
+ *
+ * @param code value allocated by authority.
+ * @param create whether to create {@link IdentifiedObject} from {@link
StoredTree}.
+ * @return the object for the given code, possibly as a {@link StoredTree}
if {@code create} is {@code false}.
+ * @throws NoSuchAuthorityCodeException if the specified {@code code} was
not found.
+ * @throws FactoryException if the object creation failed for some other
reason.
+ */
+ private Object getOrCreate(final String code, final boolean create) throws
FactoryException {
/*
* Separate the authority from the rest of the code. The
CharSequences.skipWhitespaces(…)
* methods are robust to negative index and will work even if
code.indexOf(…) returned -1.
@@ -825,7 +891,7 @@ public class WKTDictionary extends GeodeticAuthorityFactory
{
* If `StoredTree`, try to replace that value by an `IdentifiedObject`
(on success) or `String` (on failure).
* Must be done under write lock because `parser` is not thread-safe.
*/
- if (value instanceof StoredTree) {
+ if (create && value instanceof StoredTree) {
lock.writeLock().lock();
try {
if (choices != null) {
@@ -859,12 +925,7 @@ public class WKTDictionary extends
GeodeticAuthorityFactory {
lock.writeLock().unlock();
}
}
- if (value instanceof IdentifiedObject) {
- return (IdentifiedObject) value;
- } else {
- // Exception message saved in a previous invocation of this method.
- throw new FactoryException(String.valueOf(value));
- }
+ return value;
}
/**
diff --git
a/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/WKTDictionaryTest.java
b/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/WKTDictionaryTest.java
index 56001f0..76c6dae 100644
---
a/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/WKTDictionaryTest.java
+++
b/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/WKTDictionaryTest.java
@@ -17,7 +17,12 @@
package org.apache.sis.io.wkt;
import java.util.Set;
+import java.util.Map;
import java.util.Arrays;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.function.Consumer;
+import java.util.function.BiFunction;
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.IOException;
@@ -60,7 +65,13 @@ public final strictfp class WKTDictionaryTest extends
TestCase {
factory.load(source);
}
/*
- * TEST code space should be fist because it is the most frequently
used
+ * The `load(…)` method should detect duplicated WKT elements and use
references
+ * to unique instances of nodes such as "AngleUnit["Degree",
0.0174532925199433]]".
+ * Following line verifies if the trees of WKT elements indeed share
common nodes.
+ */
+ new SharedValuesCheck().verify(factory);
+ /*
+ * "TEST" code space should be fist because it is the most frequently
used
* in the test file. The authority should be "TEST" for the same
reason.
* Codes can be in any order. Code spaces are omitted when there is no
ambiguity.
*/
@@ -76,6 +87,12 @@ public final strictfp class WKTDictionaryTest extends
TestCase {
assertSame(codes, factory.getAuthorityCodes(GeodeticCRS.class));
// Test sharing.
assertSame(codes, factory.getAuthorityCodes(GeographicCRS.class));
// Test caching.
/*
+ * Test descriptions before CRS creation.
+ * Implementation fetches them from `StoredTree` instances.
+ */
+ assertEquals("North_Pole_Stereographic",
factory.getDescriptionText("ESRI::102018").toString());
+ assertEquals("South_Pole_Stereographic",
factory.getDescriptionText("ESRI::102021").toString());
+ /*
* Tests CRS creation.
*/
verifyCRS(factory.createProjectedCRS ( "102018"),
"North_Pole_Stereographic", +90);
@@ -83,6 +100,12 @@ public final strictfp class WKTDictionaryTest extends
TestCase {
verifyCRS(factory.createGeographicCRS("TEST: :102021"), "Anguilla
1957");
verifyCRS(factory.createGeographicCRS("TEST:v2:102021"), "Anguilla
1957 (bis)");
/*
+ * Test descriptions after CRS creation.
+ * Implementation fetches them from `IdentifiedObject` instances.
+ */
+ assertEquals("North_Pole_Stereographic",
factory.getDescriptionText("ESRI::102018").toString());
+ assertEquals("South_Pole_Stereographic",
factory.getDescriptionText("ESRI::102021").toString());
+ /*
* Test creation of CRS having errors.
* - Verify error index.
*/
@@ -91,6 +114,72 @@ public final strictfp class WKTDictionaryTest extends
TestCase {
}
/**
+ * Verifies that there is no duplicated nodes in the {@link StoredTree}s.
+ * When a WKT element is repeated often (e.g. "AngleUnit["Degree",
0.0174532925199433]]"),
+ * only one {@link org.apache.sis.io.wkt.StoredTree.Node} instance should
be created and shared by all trees.
+ */
+ private static final class SharedValuesCheck implements Consumer<Object>,
BiFunction<Integer,Integer,Integer> {
+ /**
+ * Counter of number of occurrences of each instance. Keys may be
{@link String},
+ * {@link Long}, {@link Double} or {@code StoredTree.Node} instances
among others.
+ * Values are number of occurrences.
+ */
+ private final Map<Object,Integer> counts = new IdentityHashMap<>(90);
+
+ /**
+ * Verifies all trees in the given factory.
+ */
+ final void verify(final WKTDictionary factory) {
+ factory.forEachValue(this);
+ assertEquals("Some values are equal but distinct instances. A
single instance should be shared.",
+ new HashSet<>(counts.keySet()).size(), counts.size());
+ /*
+ * Verify the number of occurrences of a few values. Note that the
same string representation of keys
+ * value may appear twice: once because the value was already a
`String`, and once because the value
+ * was a `StoredTree.Node` with the same string representation.
+ *
+ * The `expected` values below are empirical values and may need
to be updated if the content of
+ * `ExtraCRS.txt` test file is modified.
+ */
+ for (final Map.Entry<Object,Integer> entry : counts.entrySet()) {
+ final String key = entry.getKey().toString();
+ final int expected;
+ switch (key) {
+ case "Cartesian": expected = 2; break;
+ case "north": expected = 6; break;
+ case "Degree": expected = 6; break;
+ case "Latitude of natural origin": {
+ /*
+ * There is 2 parameters with that string value, but
those two parameters are
+ * distinct instances because they have different
parameter values (90° and -90°).
+ */
+ expected = (entry.getKey() instanceof String) ? 2 : 1;
+ break;
+ }
+ default: continue;
+ }
+ assertEquals(key, expected, entry.getValue().intValue());
+ }
+ }
+
+ /**
+ * Invoked for each value in a WKT element. This method counts the
number of occurrences of each
+ * distinct instance, separated by identity comparison (not by {@link
Object#equals(Object)}).
+ */
+ @Override public void accept(final Object value) {
+ if (value instanceof StoredTree) {
+ ((StoredTree) value).forEachValue(this);
+ }
+ counts.merge(value, 1, this);
+ }
+
+ /** Invoked for incrementing a value in the {@link #counts} map. */
+ @Override public Integer apply(final Integer oldValue, final Integer
value) {
+ return oldValue + value;
+ }
+ }
+
+ /**
* Verifies a projected CRS.
*
* @param crs the CRS to verify.
