sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1595377 - in /sis/branches/JDK8/core: sis-feature/src/main/java/org/apache/sis/feature/ sis-utility/src/main/java/org/apache/sis/util/
Date Fri, 16 May 2014 22:33:11 GMT
Author: desruisseaux
Date: Fri May 16 22:33:10 2014
New Revision: 1595377

URL: http://svn.apache.org/r1595377
Log:
Attempt to make DefaultFeature more robust.

Added:
    sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java
  (with props)
Modified:
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java?rev=1595377&r1=1595376&r2=1595377&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
[UTF-8] Fri May 16 22:33:10 2014
@@ -77,6 +77,9 @@ public class DefaultAssociation extends 
         ArgumentChecks.ensureNonNull("role", role);
         this.role  = role;
         this.value = value;
+        if (value != null) {
+            ensureValid(role.getValueType(), value.getType());
+        }
     }
 
     /**
@@ -135,17 +138,23 @@ public class DefaultAssociation extends 
      */
     public void setValue(final DefaultFeature value) {
         if (value != null) {
-            final DefaultFeatureType base = role.getValueType();
-            final DefaultFeatureType type = value.getType();
-            if (base != type && !base.maybeAssignableFrom(type)) {
-                throw new IllegalArgumentException(
-                        Errors.format(Errors.Keys.IllegalArgumentClass_3, getName(), base.getName(),
type.getName()));
-            }
+            ensureValid(role.getValueType(), value.getType());
         }
         this.value = value;
     }
 
     /**
+     * Ensures that storing a feature of the given type is valid for an association
+     * expecting the given base type.
+     */
+    private void ensureValid(final DefaultFeatureType base, final DefaultFeatureType type)
{
+        if (base != type && !base.maybeAssignableFrom(type)) {
+            throw new IllegalArgumentException(
+                    Errors.format(Errors.Keys.IllegalArgumentClass_3, getName(), base.getName(),
type.getName()));
+        }
+    }
+
+    /**
      * Verifies if the current association value mets the constraints defined by the association
role.
      * This method returns {@linkplain org.apache.sis.metadata.iso.quality.DefaultDataQuality#getReports()
      * reports} for all constraint violations found, if any.

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java?rev=1595377&r1=1595376&r2=1595377&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
[UTF-8] Fri May 16 22:33:10 2014
@@ -20,11 +20,13 @@ import java.util.Map;
 import java.util.HashMap;
 import java.util.ConcurrentModificationException;
 import java.io.Serializable;
+import org.opengis.util.GenericName;
 import org.opengis.metadata.quality.DataQuality;
 import org.opengis.metadata.maintenance.ScopeCode;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.collection.Containers;
+import org.apache.sis.util.CorruptedObjectException;
 
 
 /**
@@ -64,6 +66,21 @@ public class DefaultFeature implements S
     private static final long serialVersionUID = 6594295132544357870L;
 
     /**
+     * A {@link #valuesKind} flag meaning that the {@link #properties} map contains raw values.
+     */
+    private static final byte VALUES = 0; // Must be zero, because we want it to be 'valuesKind'
default value.
+
+    /**
+     * A {@link #valuesKind} flag meaning that the {@link #properties} map contains {@link
Property} instances.
+     */
+    private static final byte PROPERTIES = 1;
+
+    /**
+     * A {@link #valuesKind} flag meaning that the {@link #properties} map is invalid.
+     */
+    private static final byte CORRUPTED = 2;
+
+    /**
      * Information about the feature (name, characteristics, <i>etc.</i>).
      */
     private final DefaultFeatureType type;
@@ -76,19 +93,19 @@ public class DefaultFeature implements S
      * requested. The intend is to reduce the amount of allocated objects as much as possible,
because
      * typical SIS applications may create a very large amount of features.
      *
-     * @see #propertiesAreInstantiated
+     * @see #valuesKind
      */
     private final Map<String, Object> properties;
 
     /**
-     * {@code true} if the values in the {@link #properties} map are {@link Property} instances,
-     * or {@code false} if the map contains only the "raw" property values.
+     * {@link #PROPERTIES} if the values in the {@link #properties} map are {@link Property}
instances,
+     * or {@link #VALUES} if the map contains only the "raw" property values.
      *
-     * <p>This field is initially {@code false}, and will be set to {@code true} only
if at least one
-     * {@code Property} instance has been requested. In such case, all property values will
have been
+     * <p>This field is initially {@code VALUES}, and will be set to {@code PROPERTIES}
only if at least
+     * one {@code Property} instance has been requested. In such case, all property values
will have been
      * wrapped into their appropriate {@code Property} instance.</p>
      */
-    private boolean propertiesAreInstantiated;
+    private byte valuesKind;
 
     /**
      * Creates a new feature of the given type.
@@ -144,9 +161,12 @@ public class DefaultFeature implements S
          * Wraps values in Property objects for all entries in the properties map,
          * if not already done. This operation is execute at most once per feature.
          */
-        if (!propertiesAreInstantiated) {
-            propertiesAreInstantiated = true;
+        if (valuesKind != PROPERTIES) {
             if (!properties.isEmpty()) { // The map is typically empty when this method is
first invoked.
+                if (valuesKind != VALUES) {
+                    throw new CorruptedObjectException(toString());
+                }
+                valuesKind = CORRUPTED;
                 for (final Map.Entry<String, Object> entry : properties.entrySet())
{
                     final String key      = entry.getKey();
                     final Object value    = entry.getValue();
@@ -154,14 +174,18 @@ public class DefaultFeature implements S
                     final Property property;
                     if (pt instanceof DefaultAttributeType<?>) {
                         property = new DefaultAttribute<>((DefaultAttributeType<?>)
pt, value);
+                    } else if (pt instanceof DefaultAssociationRole) {
+                        property = new DefaultAssociation((DefaultAssociationRole) pt, (DefaultFeature)
value);
                     } else {
-                        throw new UnsupportedOperationException(); // TODO
+                        // Should never happen, unless the user gave us some mutable FeatureType.
+                        throw new IllegalStateException(Errors.format(Errors.Keys.UnknownType_1,
pt));
                     }
                     if (entry.setValue(property) != value) {
                         throw new ConcurrentModificationException(key);
                     }
                 }
             }
+            valuesKind = PROPERTIES; // Set only on success.
         }
         return getPropertyInstance(name);
     }
@@ -171,13 +195,16 @@ public class DefaultFeature implements S
      * map contains {@code Property} instances (as opposed to their value).
      */
     private Property getPropertyInstance(final String name) throws IllegalArgumentException
{
+        assert valuesKind == PROPERTIES : valuesKind;
         Property property = (Property) properties.get(name);
         if (property == null) {
             final PropertyType pt = getPropertyType(name);
             if (pt instanceof DefaultAttributeType<?>) {
                 property = new DefaultAttribute<>((DefaultAttributeType<?>) pt);
+            } else if (pt instanceof DefaultAssociationRole) {
+                property = new DefaultAssociation((DefaultAssociationRole) pt);
             } else {
-                throw new UnsupportedOperationException(); // TODO
+                throw new IllegalArgumentException(unsupportedPropertyType(pt.getName()));
             }
             replace(name, null, property);
         }
@@ -186,31 +213,35 @@ public class DefaultFeature implements S
 
     /**
      * Returns the value for the property of the given name.
-     * This convenience method is equivalent to the following code,
-     * except that this method is potentially more efficient:
+     * This convenience method is equivalent to the following steps:
      *
-     * {@preformat java
-     *     return getProperty(name).getValue();
-     * }
+     * <ul>
+     *   <li>Get the property of the given name.</li>
+     *   <li>Delegates to {@link DefaultAttribute#getValue()} or {@link DefaultAssociation#getValue()},
+     *       depending on the property type.
+     * </ul>
      *
      * @param  name The property name.
      * @return The value for the given property, or {@code null} if none.
-     * @throws IllegalArgumentException If the given argument is not a property name of this
feature.
+     * @throws IllegalArgumentException If the given argument is not an attribute or association
name of this feature.
      *
      * @see DefaultAttribute#getValue()
      */
     public Object getPropertyValue(final String name) throws IllegalArgumentException {
         ArgumentChecks.ensureNonNull("name", name);
         final Object element = properties.get(name);
+        final GenericName unsupported;
         if (element != null) {
-            if (!propertiesAreInstantiated) {
+            if (valuesKind == VALUES) {
                 return element;
             } else if (element instanceof DefaultAttribute<?>) {
                 return ((DefaultAttribute<?>) element).getValue();
             } else if (element instanceof DefaultAssociation) {
                 return ((DefaultAssociation) element).getValue();
+            } else if (valuesKind == PROPERTIES) {
+                unsupported = ((Property) element).getName();
             } else {
-                throw new UnsupportedOperationException(Errors.format(Errors.Keys.UnknownType_1,
element.getClass()));
+                throw new CorruptedObjectException(toString());
             }
         } else if (properties.containsKey(name)) {
             return null; // Null has been explicitely set.
@@ -219,11 +250,13 @@ public class DefaultFeature implements S
             if (pt instanceof DefaultAttributeType<?>) {
                 return ((DefaultAttributeType<?>) pt).getDefaultValue();
             } else if (pt instanceof DefaultAssociationRole) {
-                return null;
+                return null; // No default value for associations.
             } else {
-                throw new UnsupportedOperationException(Errors.format(Errors.Keys.UnknownType_1,
pt.getClass()));
+                unsupported = pt.getName();
             }
         }
+        // Happen if the property for the given name is an Operation or an unknown PropertyType.
+        throw new IllegalArgumentException(unsupportedPropertyType(unsupported));
     }
 
     /**
@@ -244,7 +277,7 @@ public class DefaultFeature implements S
      */
     public void setPropertyValue(final String name, final Object value) throws IllegalArgumentException
{
         ArgumentChecks.ensureNonNull("name", name);
-        if (!propertiesAreInstantiated) {
+        if (valuesKind == VALUES) {
             final Object previous = properties.put(name, value);
             /*
              * Slight optimisation: if we replaced a previous value of the same class, then
we can skip the
@@ -253,23 +286,27 @@ public class DefaultFeature implements S
              */
             if (previous == null || (value != null && previous.getClass() != value.getClass()))
{
                 final PropertyType pt = type.getProperty(name);
-                if (!isValidAttributeValue(pt, value)) {
+                final Object illegalType = verifyType(pt, value);
+                if (illegalType != null) {
                     replace(name, value, previous); // Restore the previous value.
                     if (pt == null) {
                         throw new IllegalArgumentException(Errors.format(
                                 Errors.Keys.PropertyNotFound_2, type.getName(), name));
                     } else {
                         throw new ClassCastException(Errors.format(
-                                Errors.Keys.IllegalPropertyClass_2, name, value.getClass()));
+                                Errors.Keys.IllegalPropertyClass_2, name, illegalType));
                     }
                 }
             }
-        } else {
+        } else if (valuesKind == PROPERTIES) {
             final Property property = getPropertyInstance(name);
             if (property instanceof DefaultAttribute<?>) {
                 setAttributeValue((DefaultAttribute<?>) property, value);
+            } else if (property instanceof DefaultAssociation) {
+                ArgumentChecks.ensureCanCast("value", DefaultFeature.class, value);
+                setAssociationValue((DefaultAssociation) property, (DefaultFeature) value);
             } else {
-                throw new UnsupportedOperationException();
+                throw new IllegalArgumentException(unsupportedPropertyType(property.getName()));
             }
         }
     }
@@ -288,29 +325,33 @@ public class DefaultFeature implements S
     }
 
     /**
-     * Returns {@code true} if the given type is an attribute type or association role,
-     * and if the given value is valid for that type or role.
+     * Returns {@code null} if the given type is an attribute type or association role,
+     * and if the given value is valid for that type or role. Otherwise returns the name
+     * of the property type for building an exception message.
      */
-    private static boolean isValidAttributeValue(final PropertyType type, final Object value)
{
+    private static Object verifyType(final PropertyType type, final Object value) {
         if (type instanceof DefaultAttributeType<?>) {
             if (value == null) {
-                return true;
+                return null;
             }
             if (((DefaultAttributeType<?>) type).getValueClass().isInstance(value))
{
-                return true;
+                return null;
             }
-        }
-        if (type instanceof DefaultAssociationRole) {
+        } else if (type instanceof DefaultAssociationRole) {
             if (value == null) {
-                return true;
+                return null;
             }
             if (value instanceof DefaultFeature) {
-                if (((DefaultAssociationRole) type).getValueType().maybeAssignableFrom(((DefaultFeature)
value).getType())) {
-                    return true;
+                final DefaultFeatureType valueType = ((DefaultFeature) value).getType();
+                if (((DefaultAssociationRole) type).getValueType().maybeAssignableFrom(valueType))
{
+                    return null;
                 }
+                return String.valueOf(valueType.getName());
             }
+        } else {
+            throw new IllegalArgumentException(unsupportedPropertyType(type.getName()));
         }
-        return false;
+        return value.getClass();
     }
 
     /**
@@ -322,7 +363,8 @@ public class DefaultFeature implements S
     private static <T> void setAttributeValue(final DefaultAttribute<T> attribute,
final Object value) {
         if (value != null) {
             final DefaultAttributeType<T> pt = attribute.getType();
-            if (!pt.getValueClass().isInstance(value)) {
+            final Class<?> base = pt.getValueClass();
+            if (!base.isInstance(value)) {
                 throw new ClassCastException(Errors.format(Errors.Keys.IllegalPropertyClass_2,
                         pt.getName(), value.getClass()));
             }
@@ -331,6 +373,30 @@ public class DefaultFeature implements S
     }
 
     /**
+     * Sets the association value after verification of its type.
+     * For a more exhaustive validation, use {@link Validator} instead.
+     */
+    private static <T> void setAssociationValue(final DefaultAssociation association,
final DefaultFeature value) {
+        if (value != null) {
+            final DefaultAssociationRole pt = association.getRole();
+            final DefaultFeatureType base = pt.getValueType();
+            final DefaultFeatureType actual = value.getType();
+            if (!base.maybeAssignableFrom(actual)) {
+                throw new IllegalArgumentException(Errors.format(Errors.Keys.IllegalPropertyClass_2,
+                        pt.getName(), actual.getName()));
+            }
+        }
+        association.setValue(value);
+    }
+
+    /**
+     * Returns the exception message for a property type which neither an attribute or an
association.
+     */
+    private static String unsupportedPropertyType(final GenericName name) {
+        return Errors.format(Errors.Keys.CanNotInstantiate_1, name);
+    }
+
+    /**
      * Verifies if all current properties met the constraints defined by the feature type.
      * This method returns {@linkplain org.apache.sis.metadata.iso.quality.DefaultDataQuality#getReports()
      * reports} for all constraint violations found, if any.

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java?rev=1595377&r1=1595376&r2=1595377&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
[UTF-8] Fri May 16 22:33:10 2014
@@ -132,6 +132,14 @@ public class DefaultFeatureType extends 
     private final List<PropertyType> properties;
 
     /**
+     * All properties, including the ones declared in the super-types.
+     * This is an unmodifiable view of the {@link #byName} values.
+     *
+     * @see #getProperties(boolean)
+     */
+    private transient Collection<PropertyType> allProperties;
+
+    /**
      * A lookup table for fetching properties by name, including the properties from super-types.
      * This map shall not be modified after construction.
      *
@@ -237,9 +245,29 @@ public class DefaultFeatureType extends 
         assignableTo = new HashSet<>(4);
         assignableTo.add(getName());
         scanPropertiesFrom(this);
-        byName       = CollectionsExt.unmodifiableOrCopy(byName);
-        indices      = CollectionsExt.unmodifiableOrCopy(indices);
-        assignableTo = CollectionsExt.unmodifiableOrCopy(assignableTo);
+        byName        = compact(byName);
+        indices       = compact(indices);
+        assignableTo  = CollectionsExt.unmodifiableOrCopy(assignableTo);
+        allProperties = byName.values();
+        if (byName instanceof HashMap<?,?>) {
+            allProperties = Collections.unmodifiableCollection(allProperties);
+        }
+    }
+
+    /**
+     * Returns a more compact representation of the given map. This method is similar to
+     * {@link CollectionsExt#unmodifiableOrCopy(Map)}, except that it does not wrap the
+     * map in an unmodifiable view. The intend is to avoid one level of indirection for
+     * performance and memory reasons (keeping in mind that we will have lot of features).
+     * This is okay if we guaranteed that the map does not escape outside this class.
+     */
+    private static <K,V> Map<K,V> compact(final Map<K,V> map) {
+        switch (map.size()) {
+            case 0:  return Collections.emptyMap();
+            case 1:  final Map.Entry<K,V> entry = map.entrySet().iterator().next();
+                     return Collections.singletonMap(entry.getKey(), entry.getValue());
+            default: return map;
+        }
     }
 
     /**
@@ -489,7 +517,7 @@ public class DefaultFeatureType extends 
      */
     public Collection<AbstractIdentifiedType> getProperties(final boolean includeSuperTypes)
{
         // TODO: temporary cast to be removed after we upgraded GeoAPI.
-        return (Collection) (includeSuperTypes ? byName.values() : properties);
+        return (Collection) (includeSuperTypes ? allProperties : properties);
     }
 
     /**

Added: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java?rev=1595377&view=auto
==============================================================================
--- sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java
(added)
+++ sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java
[UTF-8] Fri May 16 22:33:10 2014
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sis.util;
+
+
+/**
+ * May be thrown on attempt to use an object which has been corrupted by a previous operation.
+ * Apache SIS throws this exception only on a <em>best effort</em> basis, when
it detected an
+ * object in an inconsistent state <em>after</em> the original problem.
+ *
+ * <div class="note"><b>Analogy:</b>
+ * this exception has a similar goal than {@link java.util.ConcurrentModificationException}:
to reduce the risk of
+ * non-deterministic behavior at an undetermined time in the future after an event has compromised
the data integrity.
+ * Like {@code ConcurrentModificationException}, this {@code CorruptedObjectException} should
be used only to detect
+ * bugs; it would be wrong to write a program that depends on this exception for its correctness.
+ *
+ * <p>This exception is different than {@link AssertionError} in that {@code CorruptedObjectException}
is not
+ * necessarily caused by a bug in the library. An object may become corrupted because of
external factors, as
+ * illustrated in the use cases below.</p></div>
+ *
+ * Some use cases for this exception are:
+ * <ul>
+ *   <li><p><b>Attempt to use an aborted calculation:</b>
+ *   if an operation failed in the middle of a structural modification, some specific exception
(<strong>not</strong>
+ *   this {@code CorruptedObjectException}) should be thrown and the object discarded. But
if the user does not discard
+ *   the object and try to use it again, unpredictable behavior may happen. Some implementations
are robust enough for
+ *   detecting such unsafe usage: their methods may throw this {@code CorruptedObjectException}
on attempt to use the
+ *   object after the original failure.</p></li>
+ *
+ *   <li><p><b>Change in an “immutable” object:</b>
+ *   some objects are expected to be immutable. For example the same Coordinate Reference
System (CRS) instance is
+ *   typically shared by thousands of objects. However {@link org.opengis.referencing.crs.CoordinateReferenceSystem}
+ *   is an interface, Therefore, nothing prevent users from providing a mutable instance.
For example if the value
+ *   returned by {@link org.opengis.referencing.cs.CoordinateSystem#getDimension()} changes
between two invocations,
+ *   many objects that use that coordinate system will fall in an inconsistent state. If
an operation detects such
+ *   inconsistency, it may throw this {@code CorruptedObjectException}.</p></li>
+ * </ul>
+ *
+ * {@section Exception cause}
+ * Since this exception may be thrown an undetermined amount of time after the data corruption,
the root cause is
+ * often unknown at this point. Sometime a more descriptive exception has been thrown earlier,
but may have been
+ * ignored by the user.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @since   0.5
+ * @version 0.5
+ * @module
+ */
+public class CorruptedObjectException extends RuntimeException {
+    /**
+     * For cross-version compatibility.
+     */
+    private static final long serialVersionUID = -7595678373605419502L;
+
+    /**
+     * Constructs a new exception with no message.
+     */
+    public CorruptedObjectException() {
+        super();
+    }
+
+    /**
+     * Constructs a new exception with the specified detail message.
+     *
+     * @param message The detail message, or {@code null} if none.
+     */
+    public CorruptedObjectException(final String message) {
+        super(message);
+    }
+}

Propchange: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/CorruptedObjectException.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain;charset=UTF-8



Mime
View raw message