Index: trunk/src/org/openstreetmap/josm/actions/upload/DiscardTagsHook.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/upload/DiscardTagsHook.java (revision 5606) +++ trunk/src/org/openstreetmap/josm/actions/upload/DiscardTagsHook.java (revision 5608) @@ -21,4 +21,5 @@ public class DiscardTagsHook implements UploadHook { + @Override public boolean checkUpload(APIDataSet apiDataSet) { List objectsToUpload = apiDataSet.getPrimitives(); Index: trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 5606) +++ trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 5608) @@ -128,6 +128,6 @@ * If list is null, replies an empty set. * - * @param - * @param list the original collection + * @param type of data (must be one of the {@link OsmPrimitive} types + * @param set the original collection * @param type the type to filter for * @return the sub-set of OSM primitives of type type @@ -621,8 +621,10 @@ public static Collection getUninterestingKeys() { if (uninteresting == null) { - uninteresting = Main.pref.getCollection("tags.uninteresting", - Arrays.asList("source", "source_ref", "source:", "note", "comment", - "converted_by", "created_by", "watch", "watch:", "fixme", "FIXME", - "description", "attribution")); + LinkedList l = new LinkedList(Arrays.asList( + "source", "source_ref", "source:", "note", "comment", + "converted_by", "watch", "watch:", "fixme", "FIXME", + "description", "attribution")); + l.addAll(getDiscardableKeys()); + uninteresting = Main.pref.getCollection("tags.uninteresting", l); } return uninteresting; @@ -967,5 +969,5 @@ /*----------------- * OTHER METHODS - *----------------/ + *----------------*/ /** @@ -1075,5 +1077,5 @@ /** * Loads (clone) this primitive from provided PrimitiveData - * @param data + * @param data The object which should be cloned */ public void load(PrimitiveData data) { @@ -1091,8 +1093,12 @@ /** * Save parameters of this primitive to the transport object - * @return + * @return The saved object data */ public abstract PrimitiveData save(); + /** + * Save common parameters of primitives to the transport object + * @param data The object to save the data into + */ protected void saveCommonAttributes(PrimitiveData data) { data.setId(id); @@ -1108,4 +1114,8 @@ } + /** + * Fetch the bounding box of the primitive + * @return Bounding box of the object + */ public abstract BBox getBBox(); Index: trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java (revision 5606) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java (revision 5608) @@ -28,15 +28,27 @@ import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.tools.MultiMap; + /** * Tests if there are duplicate relations */ -public class DuplicateRelation extends Test -{ - +public class DuplicateRelation extends Test { + + /** + * Class to store one relation members and information about it + */ public static class RelMember { + /** Role of the relation member */ private String role; + + /** Type of the relation member */ private OsmPrimitiveType type; + + /** Tags of the relation member */ private Map tags; + + /** Coordinates of the relation member */ private List coor; + + /** ID of the relation member in case it is a {@link Relation} */ private long rel_id; @@ -53,46 +65,59 @@ } + /** Extract and store relation information based on the relation member + * @param src The relation member to store information about + */ public RelMember(RelationMember src) { - role=src.getRole(); - type=src.getType(); - rel_id=0; - coor=new ArrayList(); + role = src.getRole(); + type = src.getType(); + rel_id = 0; + coor = new ArrayList(); if (src.isNode()) { - Node r=src.getNode(); - tags=r.getKeys(); - coor=new ArrayList(1); + Node r = src.getNode(); + tags = r.getKeys(); + coor = new ArrayList(1); coor.add(r.getCoor()); } if (src.isWay()) { - Way r=src.getWay(); - tags=r.getKeys(); + Way r = src.getWay(); + tags = r.getKeys(); List wNodes = r.getNodes(); - coor=new ArrayList(wNodes.size()); - for (int i=0;i(wNodes.size()); + for (int i = 0; i < wNodes.size(); i++) { coor.add(wNodes.get(i).getCoor()); } } if (src.isRelation()) { - Relation r=src.getRelation(); - tags=r.getKeys(); - rel_id=r.getId(); - coor=new ArrayList(); - } - } - } - + Relation r = src.getRelation(); + tags = r.getKeys(); + rel_id = r.getId(); + coor = new ArrayList(); + } + } + } + + /** + * Class to store relation members + */ private class RelationMembers { + /** List of member objects of the relation */ public List members; - public RelationMembers(List _members) { - members=new ArrayList(_members.size()); - for (int i=0;i<_members.size();i++) { - members.add(new RelMember(_members.get(i))); - } - } + + /** Store relation information + * @param members The list of relation members + */ + public RelationMembers(List members) { + this.members = new ArrayList(members.size()); + for (int i = 0; i < members.size(); i++) { + this.members.add(new RelMember(members.get(i))); + } + } + @Override public int hashCode() { return members.hashCode(); } + @Override public boolean equals(Object obj) { @@ -103,15 +128,27 @@ } + /** + * Class to store relation data (keys are usually cleanup and may not be equal to original relation) + */ private class RelationPair { + /** Member objects of the relation */ public RelationMembers members; + /** Tags of the relation */ public Map keys; - public RelationPair(List _members,Map _keys) { - members=new RelationMembers(_members); - keys=_keys; - } + + /** Store relation information + * @param members The list of relation members + * @param keys The set of tags of the relation + */ + public RelationPair(List members, Map keys) { + this.members = new RelationMembers(members); + this.keys = keys; + } + @Override public int hashCode() { return members.hashCode()+keys.hashCode(); } + @Override public boolean equals(Object obj) { @@ -122,5 +159,8 @@ } + /** Code number of completely duplicated relation error */ protected static final int DUPLICATE_RELATION = 1901; + + /** Code number of relation with same members error */ protected static final int SAME_RELATION = 1902; @@ -131,17 +171,17 @@ MultiMap, OsmPrimitive> relations_nokeys; - /** - * Constructor - */ - public DuplicateRelation() - { + /** List of keys without useful information */ + Collection ignoreKeys = new HashSet(OsmPrimitive.getUninterestingKeys()); + + /** + * Default constructor + */ + public DuplicateRelation() { super(tr("Duplicated relations"), tr("This test checks that there are no relations with same tags and same members with same roles.")); } - - @Override - public void startTest(ProgressMonitor monitor) - { + @Override + public void startTest(ProgressMonitor monitor) { super.startTest(monitor); relations = new MultiMap(1000); @@ -150,11 +190,8 @@ @Override - public void endTest() - { + public void endTest() { super.endTest(); - for(LinkedHashSet duplicated : relations.values() ) - { - if( duplicated.size() > 1) - { + for(LinkedHashSet duplicated : relations.values()) { + if (duplicated.size() > 1) { TestError testError = new TestError(this, Severity.ERROR, tr("Duplicated relations"), DUPLICATE_RELATION, duplicated); errors.add( testError ); @@ -162,8 +199,6 @@ } relations = null; - for(LinkedHashSet duplicated : relations_nokeys.values() ) - { - if( duplicated.size() > 1) - { + for(LinkedHashSet duplicated : relations_nokeys.values()) { + if (duplicated.size() > 1) { TestError testError = new TestError(this, Severity.WARNING, tr("Relations with same members"), SAME_RELATION, duplicated); errors.add( testError ); @@ -174,12 +209,12 @@ @Override - public void visit(Relation r) - { + public void visit(Relation r) { if (!r.isUsable() || r.hasIncompleteMembers()) return; - List rMembers=r.getMembers(); - Map rkeys=r.getKeys(); - rkeys.remove("created_by"); - RelationPair rKey=new RelationPair(rMembers,rkeys); + List rMembers = r.getMembers(); + Map rkeys = r.getKeys(); + for (String key : ignoreKeys) + rkeys.remove(ignoreKeys); + RelationPair rKey = new RelationPair(rMembers, rkeys); relations.put(rKey, r); relations_nokeys.put(rMembers, r); @@ -188,8 +223,8 @@ /** * Fix the error by removing all but one instance of duplicate relations - */ - @Override - public Command fixError(TestError testError) - { + * @param testError The error to fix, must be of type {@link #DUPLICATE_RELATION} + */ + @Override + public Command fixError(TestError testError) { if (testError.getCode() == SAME_RELATION) return null; Collection sel = testError.getPrimitives(); @@ -253,10 +288,7 @@ @Override - public boolean isFixable(TestError testError) - { - if (!(testError.getTester() instanceof DuplicateRelation)) - return false; - - if (testError.getCode() == SAME_RELATION) return false; + public boolean isFixable(TestError testError) { + if (!(testError.getTester() instanceof DuplicateRelation) + || testError.getCode() == SAME_RELATION) return false; // We fix it only if there is no more than one relation that is relation member.