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 <martin.desruisse...@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