Skip to content

Commit

Permalink
AtlasDelta and AtlasDeltaGenerator Revamp (#195)
Browse files Browse the repository at this point in the history 
* Revamp AtlasDelta and AtlasDeltaGenerator. We now get GeoJSON deltas that include before and after entities. Also, the generator CLI is not working.

* ./gradlew spotlessApply

* rename base and alter to before and after

* AtlasDelta integration testing. Also, fixed a typo bug!

* spotless apply

* explain parse geojson test

* Improve log4j instructions

* Account for PR comments.

* fix abbrev

* no more abbrev.
  • Loading branch information
@hallahan @lucaspcram
hallahan authored and lucaspcram committed Aug 16, 2018
1 parent 4c554c1 commit 1686af6
Show file tree
Hide file tree
Showing 9 changed files with 285 additions and 148 deletions.
41 changes: 41 additions & 0 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,47 @@ To start contributing to your fork, the best way is to make sure that your IDE i

Once you have fixed an issue or added a new feature, it is time to submit [a pull request](#pull-request-guidelines)!

### Building

You can build a shaded JAR that will allow you to execute atlas.

#### Log4j Properties

Make sure you first have a `log4j.properties` file in `src/main/resources`.
Alternatively, you can have as a VM parameter:

```
-Dlog4j.configuration=file://<path_to_config>
```

https://github.com/osmlab/atlas-checks/blob/dev/config/log4j/log4j.properties

Then, you can build your shaded JAR with:

```
./gradlew shaded
```

From there, you can run command line tools in atlas, like the following:

```
java -cp atlas-5.1.8-SNAPSHOT-shaded.jar org.openstreetmap.atlas.geography.atlas.delta.AtlasDeltaGenerator <args...>
```

#### IntelliJ Setup

IntellJ IDEA works pretty much out of the box. However, you still need to mess with log4j. First, add a
`log4j.properties` file to your project or your VM Options.

Also, you need to manually add [slf4j-simple.jar](https://mvnrepository.com/artifact/org.slf4j/slf4j-simple/1.7.25)
to your Project Module Dependencies:

File > Project Structure > Modules > Dependencies

In `atlas_main` add the the JAR that you downloaded.

![slf4j in Intellij](images/slf4j.png)

### Code formatting

The project's code is checked by Checkstyle as part of the `gradle check` step.
Expand Down
Binary file added images/slf4j.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
96 changes: 96 additions & 0 deletions .../java/org/openstreetmap/atlas/geography/atlas/delta/AtlasDeltaGeoJsonIntegrationTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package org.openstreetmap.atlas.geography.atlas.delta;

import org.hamcrest.CoreMatchers;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.openstreetmap.atlas.geography.atlas.Atlas;
import org.openstreetmap.atlas.geography.atlas.builder.text.TextAtlasBuilder;
import org.openstreetmap.atlas.streaming.compression.Decompressor;
import org.openstreetmap.atlas.streaming.resource.InputStreamResource;

import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;

/**
* @author hallahan
*/
public class AtlasDeltaGeoJsonIntegrationTest
{
private Atlas before;
private Atlas after;
private AtlasDelta delta;

@Before
public void readAtlases()
{
before = new TextAtlasBuilder()
.read(new InputStreamResource(() -> AtlasDeltaIntegrationTest.class
.getResourceAsStream("DMA_9-168-233-base.txt.gz"))
.withDecompressor(Decompressor.GZIP)
.withName("DMA_9-168-233-base.txt.gz"));
after = new TextAtlasBuilder()
.read(new InputStreamResource(() -> AtlasDeltaIntegrationTest.class
.getResourceAsStream("DMA_9-168-233-alter.txt.gz"))
.withDecompressor(Decompressor.GZIP)
.withName("DMA_9-168-233-alter.txt.gz"));
delta = new AtlasDelta(before, after, false).generate();
}

/**
* This is a basic test that should start failing if you change what the delta GeoJSON looks
* like.
*/
@Test
public void testGeoJson()
{
final String geoJson = delta.toGeoJson();
Assert.assertEquals(22424431, geoJson.length());
}

@Test
public void testRelationsGeoJson()
{
final String relationsGeoJson = delta.toRelationsGeoJson();
Assert.assertEquals(454484, relationsGeoJson.length());
}

/**
* Tries parsing the GeoJSON string. We then check a few things about it, such as if it has the
* applicable diff properties. Also, we count the number of features.
*/
@Test
public void parseGeoJson()
{
final String geoJsonStr = delta.toGeoJson();

final JsonObject geoJson = new JsonParser().parse(geoJsonStr).getAsJsonObject();
final JsonArray features = geoJson.getAsJsonArray("features");

int idx = 0;
for (; idx < features.size(); ++idx)
{
final JsonObject feature = features.get(idx).getAsJsonObject();
final JsonObject properties = feature.getAsJsonObject("properties");

final JsonElement diffVal = properties.get("diff");
Assert.assertNotNull(diffVal);
final JsonElement diffReasonVal = properties.get("diff:reason");
Assert.assertNotNull(diffReasonVal);
final JsonElement diffTypeVal = properties.get("diff:type");
Assert.assertNotNull(diffTypeVal);

// a diff property should be before or after.
Assert.assertThat(diffVal.getAsString(),
CoreMatchers.anyOf(CoreMatchers.is("BEFORE"), CoreMatchers.is("AFTER")));

// Make sure we have a reason and a type.
Assert.assertTrue(diffReasonVal.getAsString().length() > 0);
Assert.assertTrue(diffTypeVal.getAsString().length() > 0);
}

Assert.assertEquals(47646, idx);
}
}
6 changes: 3 additions & 3 deletions ...ionTest/java/org/openstreetmap/atlas/geography/atlas/delta/AtlasDeltaIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ public class AtlasDeltaIntegrationTest
@Test
public void testDiff()
{
final Atlas base = new TextAtlasBuilder()
final Atlas before = new TextAtlasBuilder()
.read(new InputStreamResource(() -> AtlasDeltaIntegrationTest.class
.getResourceAsStream("DMA_9-168-233-base.txt.gz"))
.withDecompressor(Decompressor.GZIP)
.withName("DMA_9-168-233-base.txt.gz"));
final Atlas alter = new TextAtlasBuilder()
final Atlas after = new TextAtlasBuilder()
.read(new InputStreamResource(() -> AtlasDeltaIntegrationTest.class
.getResourceAsStream("DMA_9-168-233-alter.txt.gz"))
.withDecompressor(Decompressor.GZIP)
.withName("DMA_9-168-233-alter.txt.gz"));
final AtlasDelta delta = new AtlasDelta(base, alter, true).generate();
final AtlasDelta delta = new AtlasDelta(before, after, true).generate();
final SortedSet<Diff> differences = delta.getDifferences();
final long size = differences.size();
final long sizeNodes = differences.stream()
Expand Down
58 changes: 29 additions & 29 deletions src/main/java/org/openstreetmap/atlas/geography/atlas/delta/AtlasDelta.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,21 @@ public class AtlasDelta implements Serializable
private static final Logger logger = LoggerFactory.getLogger(AtlasDelta.class);
private static final int COUNTER_REPORT = 100_000;

private final Atlas base;
private final Atlas alter;
private final Atlas before;
private final Atlas after;
private final SortedSet<Diff> differences;
private final boolean withGeometryMatching;
private final transient CounterWithStatistic counter;

public AtlasDelta(final Atlas base, final Atlas alter)
public AtlasDelta(final Atlas before, final Atlas after)
{
this(base, alter, false);
this(before, after, false);
}

public AtlasDelta(final Atlas base, final Atlas alter, final boolean withGeometryMatching)
public AtlasDelta(final Atlas before, final Atlas after, final boolean withGeometryMatching)
{
this.base = base;
this.alter = alter;
this.before = before;
this.after = after;
this.differences = new TreeSet<>();
this.counter = new CounterWithStatistic(logger, COUNTER_REPORT, "Processed");
this.withGeometryMatching = withGeometryMatching;
Expand All @@ -67,34 +67,34 @@ public AtlasDelta generate()
{
// Check for removed identifiers
logger.info("Looking for removed items.");
for (final AtlasEntity entity : this.base)
for (final AtlasEntity entity : this.before)
{
this.counter.increment();
if (entity.getType().entityForIdentifier(this.alter, entity.getIdentifier()) == null
&& (!(entity instanceof Edge) || !hasGoodMatch((Edge) entity, this.alter)))
if (entity.getType().entityForIdentifier(this.after, entity.getIdentifier()) == null
&& (!(entity instanceof Edge) || !hasGoodMatch((Edge) entity, this.after)))
{
this.differences.add(new Diff(entity.getType(), DiffType.REMOVED,
DiffReason.REMOVED, this.base, this.alter, entity.getIdentifier()));
DiffReason.REMOVED, this.before, this.after, entity.getIdentifier()));
}
}
// Check for added identifiers
logger.info("Looking for added items.");
for (final AtlasEntity entity : this.alter)
for (final AtlasEntity entity : this.after)
{
this.counter.increment();
if (entity.getType().entityForIdentifier(this.base, entity.getIdentifier()) == null
&& (!(entity instanceof Edge) || !hasGoodMatch((Edge) entity, this.base)))
if (entity.getType().entityForIdentifier(this.before, entity.getIdentifier()) == null
&& (!(entity instanceof Edge) || !hasGoodMatch((Edge) entity, this.before)))
{
this.differences.add(new Diff(entity.getType(), DiffType.ADDED, DiffReason.ADDED,
this.base, this.alter, entity.getIdentifier()));
this.before, this.after, entity.getIdentifier()));
}
}
logger.info("Looking for changed items.");
for (final AtlasEntity baseEntity : this.base)
for (final AtlasEntity baseEntity : this.before)
{
this.counter.increment();
final long identifier = baseEntity.getIdentifier();
final AtlasEntity alterEntity = baseEntity.getType().entityForIdentifier(this.alter,
final AtlasEntity alterEntity = baseEntity.getType().entityForIdentifier(this.after,
baseEntity.getIdentifier());
// Look only at entities that are in both Atlas.
if (alterEntity != null)
Expand All @@ -103,19 +103,19 @@ public AtlasDelta generate()
if (!baseEntity.getTags().equals(alterEntity.getTags()))
{
this.differences.add(new Diff(baseEntity.getType(), DiffType.CHANGED,
DiffReason.TAGS, this.base, this.alter, identifier));
DiffReason.TAGS, this.before, this.after, identifier));
}
else if (differentInRelation(baseEntity, alterEntity))
{
this.differences.add(new Diff(baseEntity.getType(), DiffType.CHANGED,
DiffReason.RELATION_MEMBER, this.base, this.alter, identifier));
DiffReason.RELATION_MEMBER, this.before, this.after, identifier));
}
else if (baseEntity instanceof Node)
{
if (differentNodes((Node) baseEntity, (Node) alterEntity))
{
this.differences.add(new Diff(ItemType.NODE, DiffType.CHANGED,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.base, this.alter,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.before, this.after,
identifier));
}
}
Expand All @@ -124,7 +124,7 @@ else if (baseEntity instanceof Edge)
if (differentEdges((Edge) baseEntity, (Edge) alterEntity))
{
this.differences.add(new Diff(ItemType.EDGE, DiffType.CHANGED,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.base, this.alter,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.before, this.after,
identifier));
}
}
Expand All @@ -133,7 +133,7 @@ else if (baseEntity instanceof Area)
if (differentAreas((Area) baseEntity, (Area) alterEntity))
{
this.differences.add(new Diff(ItemType.AREA, DiffType.CHANGED,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.base, this.alter,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.before, this.after,
identifier));
}
}
Expand All @@ -142,7 +142,7 @@ else if (baseEntity instanceof Line)
if (differentLines((Line) baseEntity, (Line) alterEntity))
{
this.differences.add(new Diff(ItemType.LINE, DiffType.CHANGED,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.base, this.alter,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.before, this.after,
identifier));
}
}
Expand All @@ -151,7 +151,7 @@ else if (baseEntity instanceof Point)
if (differentPoints((Point) baseEntity, (Point) alterEntity))
{
this.differences.add(new Diff(ItemType.POINT, DiffType.CHANGED,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.base, this.alter,
DiffReason.GEOMETRY_OR_TOPOLOGY, this.before, this.after,
identifier));
}
}
Expand All @@ -160,7 +160,7 @@ else if (baseEntity instanceof Relation)
if (differentRelations((Relation) baseEntity, (Relation) alterEntity))
{
this.differences.add(new Diff(ItemType.RELATION, DiffType.CHANGED,
DiffReason.RELATION_TOPOLOGY, this.base, this.alter, identifier));
DiffReason.RELATION_TOPOLOGY, this.before, this.after, identifier));
}
}
}
Expand All @@ -169,14 +169,14 @@ else if (baseEntity instanceof Relation)
return this;
}

public Atlas getAlter()
public Atlas getAfter()
{
return this.alter;
return this.after;
}

public Atlas getBase()
public Atlas getBefore()
{
return this.base;
return this.before;
}

public SortedSet<Diff> getDifferences()
Expand Down
Loading

0 comments on commit 1686af6

Please sign in to comment.