Changeset 3207 in josm for trunk/src/org/openstreetmap/josm/data

Timestamp:

2010-04-25T15:05:40+02:00 (15 years ago)

Author:

jttt

Message:

Fix #4861 Coding error reported on uploading changeset (GPS survey additions and changes to existing dataset

Location:

trunk/src/org/openstreetmap/josm/data/osm

Files:

• DataSet.java (modified) (12 diffs)
• DatasetCollection.java (modified) (4 diffs)
• OsmPrimitive.java (modified) (1 diff)
• Storage.java (modified) (9 diffs)

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -28 +28 @@
 import org.openstreetmap.josm.data.osm.event.TagsChangedEvent;
 import org.openstreetmap.josm.data.osm.event.WayNodesChangedEvent;
+import org.openstreetmap.josm.tools.Predicate;
 
 /**
@@ -52 +53 @@
     }
 
-    private Storage<OsmPrimitive> allPrimitives = new Storage<OsmPrimitive>(new IdHash());
+    private Storage<OsmPrimitive> allPrimitives = new Storage<OsmPrimitive>(new IdHash(), 16, true);
     private Map<PrimitiveId, OsmPrimitive> primitivesMap = allPrimitives.foreignKey(new IdHash());
     private List<DataSetListener> listeners = new ArrayList<DataSetListener>();
@@ -98 +99 @@
     private QuadBuckets<Node> nodes = new QuadBuckets<Node>();
 
+    private <T extends OsmPrimitive> Collection<T> getPrimitives(Predicate<OsmPrimitive> predicate) {
+        return new DatasetCollection<T>(allPrimitives, predicate);
+    }
+
     /**
      * Replies an unmodifiable collection of nodes in this dataset
@@ -104 +109 @@
      */
     public Collection<Node> getNodes() {
-        return Collections.unmodifiableCollection(nodes);
+        return getPrimitives(OsmPrimitive.nodePredicate);
     }
 
@@ -124 +129 @@
      */
     public Collection<Way> getWays() {
-        return Collections.unmodifiableCollection(ways);
+        return getPrimitives(OsmPrimitive.wayPredicate);
     }
 
@@ -134 +139 @@
      * All relations/relationships
      */
-    private Collection<Relation> relations = new LinkedList<Relation>();
+    private Collection<Relation> relations = new ArrayList<Relation>();
 
     /**
@@ -142 +147 @@
      */
     public Collection<Relation> getRelations() {
-        return Collections.unmodifiableCollection(relations);
+        return getPrimitives(OsmPrimitive.relationPredicate);
     }
 
@@ -165 +170 @@
      */
     public Collection<OsmPrimitive> allPrimitives() {
-        return Collections.unmodifiableCollection(allPrimitives);
+        return getPrimitives(OsmPrimitive.allPredicate);
     }
 
@@ -172 +177 @@
      */
     public Collection<OsmPrimitive> allNonDeletedPrimitives() {
-        return new DatasetCollection(allPrimitives, OsmPrimitive.nonDeletedPredicate);
+        return getPrimitives(OsmPrimitive.nonDeletedPredicate);
     }
 
     public Collection<OsmPrimitive> allNonDeletedCompletePrimitives() {
-        return new DatasetCollection(allPrimitives, OsmPrimitive.nonDeletedCompletePredicate);
+        return getPrimitives(OsmPrimitive.nonDeletedCompletePredicate);
     }
 
     public Collection<OsmPrimitive> allNonDeletedPhysicalPrimitives() {
-        return new DatasetCollection(allPrimitives, OsmPrimitive.nonDeletedPhysicalPredicate);
+        return getPrimitives(OsmPrimitive.nonDeletedPhysicalPredicate);
     }
 
     public Collection<OsmPrimitive> allModifiedPrimitives() {
-        return new DatasetCollection(allPrimitives, OsmPrimitive.modifiedPredicate);
+        return getPrimitives(OsmPrimitive.modifiedPredicate);
     }
 
@@ -471 +476 @@
             return;
         }
+        clearDisabled(allPrimitives());
+        for (OsmPrimitive o : osm)
+            if (o != null) {
+                o.setDisabled(true);
+            }
+    }
+
+    public void setDisabled(Collection<? extends OsmPrimitive> selection) {
         clearDisabled(nodes);
         clearDisabled(ways);
         clearDisabled(relations);
-        for (OsmPrimitive o : osm)
-            if (o != null) {
-                o.setDisabled(true);
-            }
-    }
+        for (OsmPrimitive osm : selection) {
+            osm.setDisabled(true);
+        }
+    }
+
+    /**
+     * Remove the disabled parameter from every value in the collection.
+     * @param list The collection to remove the disabled parameter from.
+     */
+    private void clearDisabled(Collection<? extends OsmPrimitive> list) {
+        for (OsmPrimitive osm : list) {
+            osm.setDisabled(false);
+        }
+    }
+
 
     public void setFiltered(Collection<? extends OsmPrimitive> selection) {
@@ -503 +526 @@
     }
 
-    public void setDisabled(Collection<? extends OsmPrimitive> selection) {
-        clearDisabled(nodes);
-        clearDisabled(ways);
-        clearDisabled(relations);
-        for (OsmPrimitive osm : selection) {
-            osm.setDisabled(true);
-        }
-    }
-
     /**
      * Remove the filtered parameter from every value in the collection.
@@ -521 +535 @@
         for (OsmPrimitive osm : list) {
             osm.setFiltered(false);
-        }
-    }
-    /**
-     * Remove the disabled parameter from every value in the collection.
-     * @param list The collection to remove the disabled parameter from.
-     */
-    private void clearDisabled(Collection<? extends OsmPrimitive> list) {
-        if (list == null)
-            return;
-        for (OsmPrimitive osm : list) {
-            osm.setDisabled(false);
         }
     }

trunk/src/org/openstreetmap/josm/data/osm/DatasetCollection.java

@@ -8 +8 @@
 import org.openstreetmap.josm.tools.Predicate;
 
-public class DatasetCollection extends AbstractCollection<OsmPrimitive> {
+public class DatasetCollection<T extends OsmPrimitive> extends AbstractCollection<T> {
 
-    private class FilterIterator implements Iterator<OsmPrimitive> {
+    private class FilterIterator implements Iterator<T> {
 
         private final Iterator<? extends OsmPrimitive> iterator;
@@ -35 +35 @@
         }
 
-        public OsmPrimitive next() {
+        @SuppressWarnings("unchecked")
+        public T next() {
             findNext();
             OsmPrimitive old = current;
             current = null;
-            return old;
+            return (T)old;
         }
 
@@ -56 +57 @@
 
     @Override
-    public Iterator<OsmPrimitive> iterator() {
+    public Iterator<T> iterator() {
         return new FilterIterator(primitives.iterator());
     }
@@ -63 +64 @@
     public int size() {
         int size = 0;
-        Iterator<OsmPrimitive> it = iterator();
+        Iterator<T> it = iterator();
         while (it.hasNext()) {
             size++;

trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -438 +438 @@
         }
     };
+
+    public static Predicate<OsmPrimitive> nodePredicate = new Predicate<OsmPrimitive>() {
+        public boolean evaluate(OsmPrimitive primitive) {
+            return primitive.getClass() == Node.class;
+        }
+    };
+
+    public static Predicate<OsmPrimitive> wayPredicate = new Predicate<OsmPrimitive>() {
+        public boolean evaluate(OsmPrimitive primitive) {
+            return primitive.getClass() == Way.class;
+        }
+    };
+
+    public static Predicate<OsmPrimitive> relationPredicate = new Predicate<OsmPrimitive>() {
+        public boolean evaluate(OsmPrimitive primitive) {
+            return primitive.getClass() == Relation.class;
+        }
+    };
+
+    public static Predicate<OsmPrimitive> allPredicate = new Predicate<OsmPrimitive>() {
+        public boolean evaluate(OsmPrimitive primitive) {
+            return true;
+        }
+    };
+
 
     /**

trunk/src/org/openstreetmap/josm/data/osm/Storage.java

@@ -23 +23 @@
 import java.util.AbstractSet;
 import java.util.Collection;
+import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 import java.util.Map;
@@ -92 +93 @@
     private transient volatile int modCount = 0;
     private float loadFactor = 0.6f;
+    private final boolean safeIterator;
+    private boolean arrayCopyNecessary;
 
     public Storage() {
@@ -98 +101 @@
 
     public Storage(int capacity) {
-        this(Storage.<T>defaultHash(), capacity);
+        this(Storage.<T>defaultHash(), capacity, false);
     }
 
     public Storage(Hash<? super T,? super T> ha) {
-        this(ha, 16);
-    }
-
-    public Storage(Hash<? super T, ? super T> ha, int capacity) {
+        this(ha, 16, false);
+    }
+
+    public Storage(Hash<? super T, ? super T> ha, int capacity, boolean safeIterator) {
         this.hash = ha;
         int cap = 1 << (int)(Math.ceil(Math.log(capacity/loadFactor) / Math.log(2)));
         data = new Object[cap];
         mask = data.length - 1;
+        this.safeIterator = safeIterator;
+    }
+
+    private void copyArray() {
+        if (arrayCopyNecessary) {
+            Object[] newData = new Object[data.length];
+            System.arraycopy(data, 0, newData, 0, data.length);
+            data = newData;
+            arrayCopyNecessary = false;
+        }
     }
 
     // --------------- Collection implementation ------------------------
     @Override
-    public int size() {
+    public synchronized int size() {
         return size;
     }
 
     @Override
-    public Iterator<T> iterator() {
-        return new Iter();
-    }
-
-    public @Override boolean contains(Object o) {
+    public synchronized Iterator<T> iterator() {
+        if (safeIterator) {
+            arrayCopyNecessary = true;
+            return new SafeReadonlyIter(data);
+        } else
+            return new Iter();
+
+    }
+
+    public synchronized @Override boolean contains(Object o) {
         int bucket = getBucket(hash, (T)o);
         return bucket >= 0;
     }
 
-    public @Override boolean add(T t) {
+    public synchronized @Override boolean add(T t) {
         T orig = putUnique(t);
         return orig == t;
     }
 
-    public @Override boolean remove(Object o) {
+    public synchronized @Override boolean remove(Object o) {
         T orig = removeElem((T)o);
         return orig != null;
     }
 
-    public @Override void clear() {
+    public synchronized @Override void clear() {
+        copyArray();
         modCount++;
         size = 0;
@@ -146 +165 @@
     }
 
-    public @Override int hashCode() {
+    public synchronized @Override int hashCode() {
         int h = 0;
         for (T t : this) {
@@ -156 +175 @@
     // ----------------- Extended API ----------------------------
 
-    public T put(T t) {
+    public synchronized T put(T t) {
+        copyArray();
         modCount++;
         ensureSpace();
@@ -173 +193 @@
     }
 
-    public T get(T t) {
+    public synchronized T get(T t) {
         int bucket = getBucket(hash, t);
         return bucket < 0 ? null : (T)data[bucket];
     }
 
-    public T putUnique(T t) {
+    public synchronized T putUnique(T t) {
+        copyArray();
         modCount++;
         ensureSpace();
@@ -193 +214 @@
     }
 
-    public T removeElem(T t) {
+    public synchronized T removeElem(T t) {
+        copyArray();
         modCount++;
         int bucket = getBucket(hash, t);
@@ -381 +403 @@
     }
 
+    private final class SafeReadonlyIter implements Iterator<T> {
+        final Object[] data;
+        int slot = 0;
+
+        SafeReadonlyIter(Object[] data) {
+            this.data = data;
+        }
+
+        public boolean hasNext() {
+            align();
+            return slot < data.length;
+        }
+
+        public T next() {
+            if (!hasNext()) throw new NoSuchElementException();
+            return (T)data[slot++];
+        }
+
+        public void remove() {
+            throw new UnsupportedOperationException();
+        }
+
+        private void align() {
+            while (slot < data.length && data[slot] == null) {
+                slot++;
+            }
+        }
+    }
+
+
     private final class Iter implements Iterator<T> {
         private final int mods;
@@ -410 +462 @@
 
         private void align() {
-            if (mods != modCount) {
-                System.err.println("Warning: ConcurrentModification");
-                Thread.dumpStack();
-                //throw new ConcurrentModificationException();
-            }
+            if (mods != modCount)
+                throw new ConcurrentModificationException();
             while (slot < data.length && data[slot] == null) {
                 slot++;