You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2022/11/15 09:23:25 UTC
[sis] 02/04: Better error messages when a `ParameterValue` can not be unmarshalled because of missing `ParameterDescriptor`.
This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sis.git
commit 60d7377d331e5ec2aaa3056bde6cfcc0804c2550
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Sat Oct 29 14:30:58 2022 +0200
Better error messages when a `ParameterValue` can not be unmarshalled because of missing `ParameterDescriptor`.
---
.../sis/internal/jaxb/SpecializedIdentifier.java | 2 +
.../org/apache/sis/internal/metadata/Merger.java | 6 +--
.../referencing/CC_GeneralOperationParameter.java | 49 ++++++++++++++++------
.../jaxb/referencing/CC_GeneralParameterValue.java | 11 +----
.../jaxb/referencing/CC_OperationMethod.java | 11 +----
.../sis/parameter/AbstractParameterDescriptor.java | 4 +-
.../sis/parameter/DefaultParameterDescriptor.java | 4 +-
.../sis/parameter/DefaultParameterValue.java | 7 +++-
8 files changed, 54 insertions(+), 40 deletions(-)
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/jaxb/SpecializedIdentifier.java b/core/sis-metadata/src/main/java/org/apache/sis/internal/jaxb/SpecializedIdentifier.java
index b5e50f0f3e..d0d15075ac 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/internal/jaxb/SpecializedIdentifier.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/jaxb/SpecializedIdentifier.java
@@ -60,6 +60,7 @@ public final class SpecializedIdentifier<T> implements ReferenceIdentifier, Clon
*
* @see #getAuthority()
*/
+ @SuppressWarnings("serial") // Not statically typed as Serializable.
private final IdentifierSpace<T> authority;
/**
@@ -72,6 +73,7 @@ public final class SpecializedIdentifier<T> implements ReferenceIdentifier, Clon
* @see #getValue()
* @see #getCode()
*/
+ @SuppressWarnings("serial") // Not statically typed as Serializable.
T value;
/**
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Merger.java b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Merger.java
index 852a542b3a..f35c8ee070 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Merger.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Merger.java
@@ -146,13 +146,13 @@ public class Merger {
* we are going to merge those two metadata and verify that we are not in an infinite loop.
* We will also verify that the target metadata does not contain a source, or vice-versa.
*/
- { // For keeping 'sourceDone' and 'targetDone' more local.
+ { // For keeping `sourceDone` and `targetDone` more local.
final Boolean sourceDone = done.put(source, Boolean.FALSE);
final Boolean targetDone = done.put(target, Boolean.TRUE);
if (sourceDone != null || targetDone != null) {
if (Boolean.FALSE.equals(sourceDone) && Boolean.TRUE.equals(targetDone)) {
/*
- * At least, the 'source' and 'target' status are consistent. Pretend that we have already
+ * At least, the `source` and `target` status are consistent. Pretend that we have already
* merged those metadata since actually the merge operation is probably underway by the caller.
*/
return true;
@@ -265,7 +265,7 @@ distribute: while (it.hasNext()) {
if (!success) {
if (dryRun) break;
merge(target, propertyName, sourceValue, targetValue);
- success = true; // If no exception has been thrown by 'merged', assume the conflict solved.
+ success = true; // If no exception has been thrown by `merged`, assume the conflict solved.
}
}
}
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralOperationParameter.java b/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralOperationParameter.java
index fec9139108..bc58625c90 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralOperationParameter.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralOperationParameter.java
@@ -39,11 +39,15 @@ import org.apache.sis.parameter.DefaultParameterValueGroup;
import org.apache.sis.parameter.Parameters;
import org.apache.sis.referencing.NamedIdentifier;
import org.apache.sis.referencing.IdentifiedObjects;
+import org.apache.sis.referencing.GeodeticException;
import org.apache.sis.util.collection.Containers;
import org.apache.sis.util.CorruptedObjectException;
import org.apache.sis.internal.util.CollectionsExt;
import org.apache.sis.internal.jaxb.gco.PropertyType;
import org.apache.sis.internal.jaxb.Context;
+import org.apache.sis.util.resources.Errors;
+import org.apache.sis.xml.IdentifiedObject;
+import org.apache.sis.xml.IdentifierSpace;
/**
@@ -62,7 +66,7 @@ import org.apache.sis.internal.jaxb.Context;
* </ul>
*
* @author Martin Desruisseaux (Geomatys)
- * @version 0.6
+ * @version 1.3
* @since 0.6
* @module
*/
@@ -74,7 +78,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
/**
* The properties to ignore in the descriptor parsed from GML when this descriptor is merged with a
- * pre-defined descriptor. Remarks:
+ * predefined descriptor. Remarks:
*
* <ul>
* <li>We ignore the name because the comparisons shall be performed by the caller with
@@ -108,7 +112,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
}
/**
- * Constructor for the {@link #wrap} method only.
+ * Constructor for the {@link #wrap(GeneralParameterDescriptor)} method only.
*/
private CC_GeneralOperationParameter(final GeneralParameterDescriptor parameter) {
super(parameter);
@@ -162,14 +166,35 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
/**
* Verifies that the given descriptor is non-null and contains at least a name.
* This method is used after unmarshalling.
+ * We do this validation because parameter descriptors are mandatory and SIS classes need them.
+ * So we provide an error message here instead of waiting for a {@link NullPointerException}
+ * to occur in some arbitrary place.
+ *
+ * @param descriptor the descriptor to validate.
+ * @param property the name of the property to report as missing if an exception is thrown.
+ * @throws GeodeticException if the parameters are missing or invalid.
*/
- static boolean isValid(final GeneralParameterDescriptor descriptor) {
- return descriptor != null && descriptor.getName() != null;
+ static void validate(final GeneralParameterDescriptor descriptor, String property) {
+ if (descriptor == null || descriptor.getName() == null) {
+ short key = Errors.Keys.MissingValueForProperty_1;
+ /*
+ * The exception thrown by this method must be unchecked,
+ * otherwise JAXB just reports is without propagating it.
+ */
+ if (descriptor instanceof IdentifiedObject) {
+ final String link = ((IdentifiedObject) descriptor).getIdentifierMap().get(IdentifierSpace.XLINK);
+ if (link != null) {
+ key = Errors.Keys.NotABackwardReference_1;
+ property = link;
+ }
+ }
+ throw new GeodeticException(Errors.format(key, property));
+ }
}
/**
* Returns {@code true} if the given descriptor is restricted to a constant value.
- * This constraint exists in some pre-defined map projections.
+ * This constraint exists in some predefined map projections.
*
* <div class="note"><b>Example:</b>
* the <cite>"Latitude of natural origin"</cite> parameter of <cite>"Mercator (1SP)"</cite> projection
@@ -203,7 +228,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
* <li>The descriptor for a {@code <gml:ParameterValue>} element. Those descriptors are more complete than the
* ones provided by {@code <gml:OperationParameter>} elements alone because the parameter value allows SIS
* to infer the {@code valueClass}.</li>
- * <li>A pre-defined parameter descriptor from the {@link org.apache.sis.internal.referencing.provider} package.</li>
+ * <li>A predefined parameter descriptor from the {@link org.apache.sis.internal.referencing.provider} package.</li>
* </ul>
*
* @param provided the descriptor unmarshalled from the GML document.
@@ -226,7 +251,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
} else {
/*
* Mismatched or unknown type. It should not happen with descriptors parsed by JAXB and with
- * pre-defined descriptors provided by SIS. But it could happen with a pre-defined descriptor
+ * predefined descriptors provided by SIS. But it could happen with a predefined descriptor
* found in a user-provided OperationMethod with malformed parameters.
* Return the descriptor found in the GML document as-is.
*/
@@ -243,7 +268,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
&& containsAll(complete.getIdentifiers(), provided.getIdentifiers());
if (canSubstitute && !isGroup) {
/*
- * The pre-defined or ParameterValue descriptor contains at least all the information found
+ * The predefined or ParameterValue descriptor contains at least all the information found
* in the descriptor parsed from the GML document. We can use the existing instance directly,
* assuming that the additional properties are acceptable.
*
@@ -260,7 +285,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
* be invoked recursively for each parameter in the group.
*/
final Map<String,Object> merged = new HashMap<>(expected);
- merged.putAll(actual); // May overwrite pre-defined properties.
+ merged.putAll(actual); // May overwrite predefined properties.
mergeArrays(GeneralParameterDescriptor.ALIAS_KEY, GenericName.class, provided.getAlias(), merged, complete.getName());
mergeArrays(GeneralParameterDescriptor.IDENTIFIERS_KEY, ReferenceIdentifier.class, provided.getIdentifiers(), merged, null);
if (isGroup) {
@@ -374,7 +399,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
provided.getMinimumOccurs(),
provided.getMaximumOccurs(),
// Values below this point are not provided in GML documents,
- // so they must be inferred from the pre-defined descriptor.
+ // so they must be inferred from the predefined descriptor.
valueClass,
Parameters.getValueDomain(complete),
CollectionsExt.toArray(complete.getValidValues(), valueClass),
@@ -431,7 +456,7 @@ public final class CC_GeneralOperationParameter extends PropertyType<CC_GeneralO
* Add the `provided` values before `complete` for two reasons:
* 1) Use the same insertion order than the declaration order in the GML file.
* 2) Replace `provided` instances by `complete` instances, since the latter
- * are sometime pre-defined instances defined as static final constants.
+ * are sometime predefined instances defined as static final constants.
*/
final Map<NamedIdentifier,T> c = new LinkedHashMap<>();
for (final T e : provided) c.put(toNamedIdentifier(e), e);
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralParameterValue.java b/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralParameterValue.java
index 3b0663e6ac..ba45ef344c 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralParameterValue.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_GeneralParameterValue.java
@@ -24,7 +24,6 @@ import org.opengis.parameter.GeneralParameterValue;
import org.apache.sis.parameter.DefaultParameterValue;
import org.apache.sis.parameter.DefaultParameterValueGroup;
import org.apache.sis.internal.jaxb.gco.PropertyType;
-import org.apache.sis.util.resources.Errors;
/**
@@ -32,7 +31,7 @@ import org.apache.sis.util.resources.Errors;
* package documentation for more information about JAXB and interface.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 0.6
+ * @version 1.3
* @since 0.6
* @module
*/
@@ -110,13 +109,7 @@ public final class CC_GeneralParameterValue extends PropertyType<CC_GeneralParam
* @param parameter the unmarshalled element.
*/
public void setElement(final GeneralParameterValue parameter) {
- if (!CC_GeneralOperationParameter.isValid(parameter.getDescriptor())) {
- /*
- * Descriptors are mandatory and SIS classes need them. Provide an error message
- * here instead of waiting for a NullPointerException in some arbitrary place.
- */
- throw new IllegalArgumentException(Errors.format(Errors.Keys.MissingValueForProperty_1, "operationParameter"));
- }
metadata = parameter;
+ CC_GeneralOperationParameter.validate(parameter.getDescriptor(), "<gml:operationParameter>");
}
}
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_OperationMethod.java b/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_OperationMethod.java
index d1e7624a34..6eb815ffad 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_OperationMethod.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/jaxb/referencing/CC_OperationMethod.java
@@ -32,7 +32,6 @@ import org.opengis.parameter.ParameterDescriptorGroup;
import org.opengis.referencing.operation.OperationMethod;
import org.apache.sis.internal.jaxb.Context;
import org.apache.sis.internal.jaxb.gco.PropertyType;
-import org.apache.sis.internal.metadata.Identifiers;
import org.apache.sis.internal.referencing.CoordinateOperations;
import org.apache.sis.internal.referencing.provider.MapProjection;
import org.apache.sis.parameter.DefaultParameterValue;
@@ -48,7 +47,7 @@ import org.apache.sis.util.ArraysExt;
* package documentation for more information about JAXB and interface.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.2
+ * @version 1.3
* @since 0.6
* @module
*/
@@ -108,14 +107,8 @@ public final class CC_OperationMethod extends PropertyType<CC_OperationMethod, O
* @param method the unmarshalled element.
*/
public void setElement(final DefaultOperationMethod method) {
- if (!CC_GeneralOperationParameter.isValid(method.getParameters())) {
- /*
- * Parameters are mandatory and SIS classes need them. Provide an error message
- * here instead of waiting for a NullPointerException in some arbitrary place.
- */
- throw new IllegalArgumentException(Identifiers.missingValueForProperty(method.getName(), "parameters"));
- }
metadata = method;
+ CC_GeneralOperationParameter.validate(method.getParameters(), "<gml:parameter>");
}
/**
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/parameter/AbstractParameterDescriptor.java b/core/sis-referencing/src/main/java/org/apache/sis/parameter/AbstractParameterDescriptor.java
index acc4da605f..c3b340c2ff 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/parameter/AbstractParameterDescriptor.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/parameter/AbstractParameterDescriptor.java
@@ -121,7 +121,7 @@ public abstract class AbstractParameterDescriptor extends AbstractIdentifiedObje
* The maximum number of times that values for this parameter group are required, as an unsigned short.
* Value {@code 0xFFFF} (or -1) means an unrestricted number of occurrences.
*
- * <p>We use a short because this value is usually 1 or a very small number like 2 or 3. This also serve
+ * <p>We use a short because this value is usually 1 or a very small number like 2 or 3. It also serves
* as a safety since a large number would be a bad idea with this parameter implementation.</p>
*
* <p><b>Consider this field as final!</b>
@@ -200,7 +200,7 @@ public abstract class AbstractParameterDescriptor extends AbstractIdentifiedObje
maximumOccurs = crop(descriptor.getMaximumOccurs());
}
- // NOTE: There is no 'castOrCopy' static method in this class because AbstractParameterDescriptor is abstract.
+ // NOTE: There is no `castOrCopy` static method in this class because AbstractParameterDescriptor is abstract.
// If nevertheless we choose to add such method in the future, then CC_GeneralOperationParameter.getElement()
// should be simplified.
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterDescriptor.java b/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterDescriptor.java
index 9fadb81fe2..68b7afd3e8 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterDescriptor.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterDescriptor.java
@@ -594,14 +594,12 @@ public class DefaultParameterDescriptor<T> extends AbstractParameterDescriptor i
* instance should not have been given to user yet.
*
* @param param the parameter value from which to infer the value type.
- * @return the parameter descriptor to assign to the given parameter value.
*/
@SuppressWarnings("unchecked")
- final DefaultParameterDescriptor<T> setValueClass(final DefaultParameterValue<?> param) {
+ final void setValueClass(final DefaultParameterValue<?> param) {
valueClass = (Class) Classes.findCommonClass(valueClass, CC_OperationParameter.valueClass(param));
if (valueDomain == null) {
valueDomain = CC_OperationParameter.valueDomain(param);
}
- return this;
}
}
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterValue.java b/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterValue.java
index 7defaf722e..bbc718c767 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterValue.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/parameter/DefaultParameterValue.java
@@ -1134,8 +1134,11 @@ convert: if (componentType != null) {
*
* @see #getDescriptor()
*/
- final void setDescriptor(final ParameterDescriptor<T> p) {
- descriptor = DefaultParameterDescriptor.castOrCopy(p).setValueClass(this);
+ final void setDescriptor(final ParameterDescriptor<T> descriptor) {
+ this.descriptor = descriptor;
+ if (descriptor instanceof DefaultParameterDescriptor<?>) {
+ ((DefaultParameterDescriptor<?>) descriptor).setValueClass(this);
+ }
/*
* A previous version was doing `assert descriptor.getValueClass().isInstance(value)`
* where the value class was inferred by `DefaultParameterDescriptor()`. But it does