GIScience / oshdb

OpenStreetMap History Data Analysis Framework
https://ohsome.org
GNU Lesser General Public License v3.0
110 stars 18 forks source link

Add contribution type "revert" #507

Open SlowMo24 opened 1 year ago

SlowMo24 commented 1 year ago

Pascal Neis has published a blog about reverted edits: https://www.openstreetmap.org/user/pitscheplatsch/diary/401668 . One of the comments is a request to show "my reverted edits". While this can already be done in OSHDB (using group by element, i.e. adapting https://gitlab.gistools.geog.uni-heidelberg.de/giscience/deleted_map/-/blob/main/extractDeleted/src/main/java/org/giscience/extractdeleted/OSHDB.java#L39-L42) and ohsome, having a dedicated contribution type could be beneficial.

Relates to #113

SlowMo24 commented 1 year ago

Here is a quick draft of how this could look like with current code although the definition of a revert slightly differs from Pascals but that should not matter: we look for geometric equality instead of member list equality and attribute revisions of minor changes to the overarching object instead of the underlying e.g. Nodes.

oshdb.getContributionView()
               // .filter...
                .filter((OSMContribution contrib) -> !contrib.getContributionTypes().isEmpty())
                .groupByEntity()
                .filter((List<OSMContribution> contribList) -> contribList.size()>1)
                .flatMap((List<OSMContribution> contribList) -> {
                    List<OSMContribution> reverts = new ArrayList<>();
                    for(int i=1; i<contribList.size(); i++){
                        OSMContribution beforeContrib = contribList.get(i-1);
                        OSMEntity beforeEntity = beforeContrib.getEntityBefore();
                        OSMContribution revertSuspect = contribList.get(i);
                        OSMEntity afterEntity = revertSuspect.getEntityAfter();
                        //check for creation + deletion (we should probably add a time constraint here because any deleted object with exactly two versions will be flagged here)
                        if((beforeContrib.is(ContributionType.CREATION) && (beforeEntity==null || !beforeEntity.isVisible()))&&
                                (revertSuspect.is(ContributionType.DELETION) && !afterEntity.isVisible())){
                            reverts.add(revertSuspect);
                        }else if (beforeEntity.getTags()==afterEntity.getTags() &&
                                beforeContrib.getGeometryUnclippedBefore().equalsExact(revertSuspect.getGeometryUnclippedAfter())){
                            reverts.add(revertSuspect);
                        }
                    }
                    return reverts;
                })

Issue #499 comes up again in line 3 above.

SlowMo24 commented 1 year ago

This is not the right place but currently the best one:

The blog only looks at X->Y->X=revert but of course this could be increased to arbitrary complexity (e.g. X->Y->Z->X=revert). Yet, this should be sufficient for a basic usage, especially since the respective Changeset should have some comments or description. But a user could e.g. change the member of a relation without knowing about the to-be-reverted-because-it-is-bad relation.

So maybe we should distinguish

reverting a full changesets -> true revert revert outside changeset -> edit war

tyrasd commented 1 year ago

As you identified by yourself, one issue here is that such a functionality is not very clearly defined and different interpretations of a "revert" exist (in addition to the cases mentioned above, even partial reverts of e.g. only the tags or geometry of an object could be considered as one).

Additionally, from a technical point of view, the internal mechanisms of the OSHDB-API currently only ever look at a pair of OSM entities when dealing with contributions, i.e. the before and after state. Extending this to at least a third state (before-before) would add quite a large level of complexity. And as this is already implementable with a relatively short OSHDB-API query, I think the additional benefit would be small.