Changeset 10346 in josm for trunk/src/org/openstreetmap

Timestamp:

2016-06-08T23:31:30+02:00 (8 years ago)

Author:

Don-vip

Message:

fix #12936 - Do not use clone() on Dataset (patch by michael2402) - gsoc-core

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/DataSource.java (modified) (1 diff)
• data/osm/DataSet.java (modified) (2 diffs)
• gui/dialogs/layer/DuplicateAction.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/data/DataSource.java

@@ -37 +37 @@
         this.bounds = bounds;
         this.origin = origin;
+    }
+
+    /**
+     * Cosntructs a new {@link DataSource}
+     * @param source The source to copy the data from.
+     * @since 10346
+     */
+    public DataSource(DataSource source) {
+        this(source.bounds, source.origin);
     }
 

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

@@ -138 +138 @@
 
     /**
+     * Creates a new {@link DataSet}.
+     * @param copyFrom An other {@link DataSet} to copy the contents of this dataset from.
+     * @since 10346
+     */
+    public DataSet(DataSet copyFrom) {
+        this();
+        copyFrom.getReadLock().lock();
+        try {
+            Map<OsmPrimitive, OsmPrimitive> primMap = new HashMap<>();
+            for (Node n : copyFrom.nodes) {
+                Node newNode = new Node(n);
+                primMap.put(n, newNode);
+                addPrimitive(newNode);
+            }
+            for (Way w : copyFrom.ways) {
+                Way newWay = new Way(w);
+                primMap.put(w, newWay);
+                List<Node> newNodes = new ArrayList<>();
+                for (Node n: w.getNodes()) {
+                    newNodes.add((Node) primMap.get(n));
+                }
+                newWay.setNodes(newNodes);
+                addPrimitive(newWay);
+            }
+            // Because relations can have other relations as members we first clone all relations
+            // and then get the cloned members
+            for (Relation r : copyFrom.relations) {
+                Relation newRelation = new Relation(r, r.isNew());
+                newRelation.setMembers(null);
+                primMap.put(r, newRelation);
+                addPrimitive(newRelation);
+            }
+            for (Relation r : copyFrom.relations) {
+                Relation newRelation = (Relation) primMap.get(r);
+                List<RelationMember> newMembers = new ArrayList<>();
+                for (RelationMember rm: r.getMembers()) {
+                    newMembers.add(new RelationMember(rm.getRole(), primMap.get(rm.getMember())));
+                }
+                newRelation.setMembers(newMembers);
+            }
+            for (DataSource source : copyFrom.dataSources) {
+                dataSources.add(new DataSource(source));
+            }
+            version = copyFrom.version;
+        } finally {
+            copyFrom.getReadLock().unlock();
+        }
+    }
+
+    /**
      * Returns the lock used for reading.
      * @return the lock used for reading
@@ -873 +923 @@
     }
 
+    /**
+     * Return a copy of this dataset
+     * @deprecated Use the copy constructor instead. Remove in July 2016
+     */
+    @Deprecated
     @Override
     public DataSet clone() {
-        getReadLock().lock();
-        try {
-            DataSet ds = (DataSet) super.clone();
-            Main.addProjectionChangeListener(ds);
-            Map<OsmPrimitive, OsmPrimitive> primMap = new HashMap<>();
-            for (Node n : nodes) {
-                Node newNode = new Node(n);
-                primMap.put(n, newNode);
-                ds.addPrimitive(newNode);
-            }
-            for (Way w : ways) {
-                Way newWay = new Way(w);
-                primMap.put(w, newWay);
-                List<Node> newNodes = new ArrayList<>();
-                for (Node n: w.getNodes()) {
-                    newNodes.add((Node) primMap.get(n));
-                }
-                newWay.setNodes(newNodes);
-                ds.addPrimitive(newWay);
-            }
-            // Because relations can have other relations as members we first clone all relations
-            // and then get the cloned members
-            for (Relation r : relations) {
-                Relation newRelation = new Relation(r, r.isNew());
-                newRelation.setMembers(null);
-                primMap.put(r, newRelation);
-                ds.addPrimitive(newRelation);
-            }
-            for (Relation r : relations) {
-                Relation newRelation = (Relation) primMap.get(r);
-                List<RelationMember> newMembers = new ArrayList<>();
-                for (RelationMember rm: r.getMembers()) {
-                    newMembers.add(new RelationMember(rm.getRole(), primMap.get(rm.getMember())));
-                }
-                newRelation.setMembers(newMembers);
-            }
-            for (DataSource source : dataSources) {
-                ds.dataSources.add(new DataSource(source.bounds, source.origin));
-            }
-            ds.version = version;
-            return ds;
-        } catch (CloneNotSupportedException e) {
-            throw new IllegalStateException(e);
-        } finally {
-            getReadLock().unlock();
-        }
+        return new DataSet(this);
     }
 

trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DuplicateAction.java

@@ -11 +11 @@
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.gui.dialogs.LayerListDialog.LayerListModel;
 import org.openstreetmap.josm.gui.help.HelpUtil;
@@ -69 +70 @@
                 i++;
             }
-            Main.main.addLayer(new OsmDataLayer(oldLayer.data.clone(), newName, null));
+            Main.main.addLayer(new OsmDataLayer(new DataSet(oldLayer.data), newName, null));
         }
     }