Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save Atlas to ProtobufAtlas similar to TextAtlas #99

Merged
merged 39 commits into from
Mar 29, 2018
Merged

Save Atlas to ProtobufAtlas similar to TextAtlas #99

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
bb7706a
Fully integrated protobuf build flow with gradle. See ECLIPSE_README.…
lucaspcram Mar 19, 2018
1890bc8
Fixed spotless application to ignore generated source code.
lucaspcram Mar 19, 2018
a03cade
Initial, naive implementation of proto specs for Tags and Points.
lucaspcram Mar 19, 2018
4e05e34
Small updates to the README files.
lucaspcram Mar 20, 2018
18611a3
Started implementation of naive ProtoAtlasBuilder
lucaspcram Mar 20, 2018
f382466
Implemented Point and Line packing.
lucaspcram Mar 21, 2018
fd9af27
Stylistic and structural changes per PR notes.
lucaspcram Mar 22, 2018
fbdaacc
Trying to get Areas working with the serializer.
lucaspcram Mar 22, 2018
c9720fe
Fixed tab indentation to use spaces in suppressions.xml
lucaspcram Mar 22, 2018
bc15496
Trying to fix the Area serialization. Currently failing due to Polygo…
lucaspcram Mar 22, 2018
a804b72
Fixed failing tests, which were due to bad copy paste.
lucaspcram Mar 22, 2018
40437f7
Integrated a few PR style suggestions.
lucaspcram Mar 22, 2018
70abe50
Added an area count to the metadata.
lucaspcram Mar 22, 2018
1eba0c9
Last of the copy paste shenanigans that I messed up. I hope. lol
lucaspcram Mar 22, 2018
d08f1a8
Ok. If this is not the last copy-paste bug then I resign.
lucaspcram Mar 22, 2018
5aa04a8
Initial drafts of Node and Edge protos.
lucaspcram Mar 22, 2018
539337e
Added unit tests for the proto converter classes.
lucaspcram Mar 23, 2018
0bfec87
Changed packages on the converter classes per PR suggestions.
lucaspcram Mar 23, 2018
5ce39d5
Forgot to stage this file.
lucaspcram Mar 23, 2018
6359a6f
Implemented support for Nodes and Edges.
lucaspcram Mar 23, 2018
ba3b136
Almost done with relations. Small bug with the OSM identifier. See un…
lucaspcram Mar 23, 2018
6db5ee7
Fixed bug with relation packing. Relations work!
lucaspcram Mar 24, 2018
a7266a6
Lots of ProtoAtlas testing.
lucaspcram Mar 26, 2018
27aa4e2
Added some commands for converting between Packed and ProtoAtlases. T…
lucaspcram Mar 26, 2018
4cd48d0
Fixed spotless formatting issue.
lucaspcram Mar 26, 2018
4a06a5d
Wrote a quick test class to investigate the proto size discrepancies.
lucaspcram Mar 26, 2018
cd9fb95
Fixed bug where tags containing delimiter characters were not deseria…
lucaspcram Mar 26, 2018
f8db3ff
Added a ProtoAtlas file suffix to FileSuffix per Mike suggestion
lucaspcram Mar 26, 2018
307b015
Changed hardcoded protoc value in build.gradle
lucaspcram Mar 27, 2018
0e19a0b
Tweak to buildflow. Gradle project refresh still requires a rebuild, but
lucaspcram Mar 28, 2018
2678b77
Small tweak to prevent Gradle Refresh from deleting the generated code.
lucaspcram Mar 28, 2018
bc6f404
Added a task to actually clear the generated code.
lucaspcram Mar 29, 2018
fe1e9a1
Slight tweak to build.gradle to fix beforeMerged
lucaspcram Mar 29, 2018
3918bb8
Hopefully ready for merge
lucaspcram Mar 29, 2018
ee43801
Finalized fixes to gradle. Everything should work now.
lucaspcram Mar 29, 2018
eeeb127
Slight change to README
lucaspcram Mar 29, 2018
0d4e3f0
Fixed checkstyle and spotless suppressions to deal with new generated…
lucaspcram Mar 29, 2018
9657bac
Final changes before merge. Hopefully we are ready to go.
lucaspcram Mar 29, 2018
79e7450
Fixed some nits.
lucaspcram Mar 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Implemented support for Nodes and Edges.
  • Loading branch information
@lucaspcram
lucaspcram committed Mar 23, 2018
commit 6359a6f208322bf236ca5c5420b9d6c286fe6f51
97 changes: 93 additions & 4 deletions src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
import org.openstreetmap.atlas.geography.atlas.AtlasMetaData;
import org.openstreetmap.atlas.geography.atlas.builder.AtlasSize;
import org.openstreetmap.atlas.geography.atlas.items.Area;
import org.openstreetmap.atlas.geography.atlas.items.Edge;
import org.openstreetmap.atlas.geography.atlas.items.Line;
import org.openstreetmap.atlas.geography.atlas.items.Node;
import org.openstreetmap.atlas.geography.atlas.items.Point;
import org.openstreetmap.atlas.geography.atlas.packed.PackedAtlas;
import org.openstreetmap.atlas.geography.atlas.packed.PackedAtlasBuilder;
Expand All @@ -23,8 +25,10 @@
import org.openstreetmap.atlas.proto.ProtoArea;
import org.openstreetmap.atlas.proto.ProtoAtlasContainer;
import org.openstreetmap.atlas.proto.ProtoAtlasContainer.Builder;
import org.openstreetmap.atlas.proto.ProtoEdge;
import org.openstreetmap.atlas.proto.ProtoLine;
import org.openstreetmap.atlas.proto.ProtoLocation;
import org.openstreetmap.atlas.proto.ProtoNode;
import org.openstreetmap.atlas.proto.ProtoPoint;
import org.openstreetmap.atlas.streaming.resource.Resource;
import org.openstreetmap.atlas.streaming.resource.WritableResource;
Expand All @@ -42,6 +46,9 @@
*/
public class ProtoAtlasBuilder
{
// File extension for naive protoatlas format
public static final String SUGGESTED_FILE_EXTENSION = ".npatlas";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding this to the FileSuffix class and using it here.


private static final Logger logger = LoggerFactory.getLogger(ProtoAtlasBuilder.class);

/**
Expand Down Expand Up @@ -70,8 +77,8 @@ public PackedAtlas read(final Resource resource)
final long numberOfPoints = protoAtlasContainer.getNumberOfPoints();
final long numberOfLines = protoAtlasContainer.getNumberOfLines();
final long numberOfAreas = protoAtlasContainer.getNumberOfAreas();
final long numberOfNodes = 0;
final long numberOfEdges = 0;
final long numberOfNodes = protoAtlasContainer.getNumberOfNodes();
final long numberOfEdges = protoAtlasContainer.getNumberOfEdges();
final long numberOfRelations = 0;
final AtlasSize atlasSize = new AtlasSize(numberOfEdges, numberOfNodes, numberOfAreas,
numberOfLines, numberOfPoints, numberOfRelations);
Expand All @@ -85,6 +92,8 @@ public PackedAtlas read(final Resource resource)
parsePoints(builder, protoAtlasContainer.getPointsList());
parseLines(builder, protoAtlasContainer.getLinesList());
parseAreas(builder, protoAtlasContainer.getAreasList());
parseNodes(builder, protoAtlasContainer.getNodesList());
parseEdges(builder, protoAtlasContainer.getEdgesList());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there no relations being added to the new packed Atlas or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're getting there. It's still in development :)

return (PackedAtlas) builder.get();
}
Expand All @@ -101,9 +110,12 @@ public void write(final Atlas atlas, final WritableResource resource)
{
final ProtoAtlasContainer.Builder protoAtlasBuilder = ProtoAtlasContainer.newBuilder();

// get the Atlas features into the ProtoAtlasBuilder
writePointsToBuilder(atlas, protoAtlasBuilder);
writeLinesToBuilder(atlas, protoAtlasBuilder);
writeAreasToBuilder(atlas, protoAtlasBuilder);
writeNodesToBuilder(atlas, protoAtlasBuilder);
writeEdgesToBuilder(atlas, protoAtlasBuilder);

final ProtoAtlasContainer protoAtlas = protoAtlasBuilder.build();
resource.writeAndClose(protoAtlas.toByteArray());
Expand All @@ -128,6 +140,21 @@ private void parseAreas(final PackedAtlasBuilder builder, final List<ProtoArea>
});
}

private void parseEdges(final PackedAtlasBuilder builder, final List<ProtoEdge> edges)
{
final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();
final ProtoTagListConverter protoTagListConverter = new ProtoTagListConverter();
edges.forEach(protoEdge ->
{
final long identifier = protoEdge.getId();
final List<Location> shapePoints = protoEdge.getShapePointsList().stream()
.map(protoLocationConverter::convert).collect(Collectors.toList());
final PolyLine geometry = new PolyLine(shapePoints);
final Map<String, String> tags = protoTagListConverter.convert(protoEdge.getTagsList());
builder.addEdge(identifier, geometry, tags);
});
}

private void parseLines(final PackedAtlasBuilder builder, final List<ProtoLine> lines)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to combine parseLines and parseAreas? The only difference is creating a PolyLine vs. a Polygon, which can be handled with a boolean flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to take a look at some of these things today. Please don't merge yet!!

{
final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();
Expand All @@ -143,16 +170,31 @@ private void parseLines(final PackedAtlasBuilder builder, final List<ProtoLine>
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseAreas and parseLines seem to be identical apart from different parameters. Would it be worth consolidating these two and passing a generic proto object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something Mike and I discussed in our 1on1. We decided for now to have them separate purely so that the design follows the API in a logically consistent way, in spite of the code reuse. Later on if it seems to be an issue we can pretty easily refactor it away.


private void parseNodes(final PackedAtlasBuilder builder, final List<ProtoNode> nodes)
{
final ProtoTagListConverter protoTagListConverter = new ProtoTagListConverter();
nodes.forEach(protoNode ->
{
final long identifier = protoNode.getId();
final Longitude longitude = Longitude.dm7(protoNode.getLocation().getLongitude());
final Latitude latitude = Latitude.dm7(protoNode.getLocation().getLatitude());
final Location geometry = new Location(latitude, longitude);
final Map<String, String> tags = protoTagListConverter.convert(protoNode.getTagsList());
builder.addNode(identifier, geometry, tags);
});
}

private void parsePoints(final PackedAtlasBuilder builder, final List<ProtoPoint> points)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment on combining parseNodes and parsePoints

{
final ProtoTagListConverter converter = new ProtoTagListConverter();
final ProtoTagListConverter protoTagListConverter = new ProtoTagListConverter();
points.forEach(protoPoint ->
{
final long identifier = protoPoint.getId();
final Longitude longitude = Longitude.dm7(protoPoint.getLocation().getLongitude());
final Latitude latitude = Latitude.dm7(protoPoint.getLocation().getLatitude());
final Location geometry = new Location(latitude, longitude);
final Map<String, String> tags = converter.convert(protoPoint.getTagsList());
final Map<String, String> tags = protoTagListConverter
.convert(protoPoint.getTagsList());
builder.addPoint(identifier, geometry, tags);
});
}
Expand Down Expand Up @@ -181,6 +223,30 @@ private void writeAreasToBuilder(final Atlas atlas, final Builder protoAtlasBuil
protoAtlasBuilder.setNumberOfAreas(numberOfAreas);
}

private void writeEdgesToBuilder(final Atlas atlas, final Builder protoAtlasBuilder)
{
long numberOfEdges = 0;
final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();
final ProtoTagListConverter protoTagListConverter = new ProtoTagListConverter();

for (final Edge edge : atlas.edges())
{
final ProtoEdge.Builder protoEdgeBuilder = ProtoEdge.newBuilder();
protoEdgeBuilder.setId(edge.getIdentifier());

final List<ProtoLocation> protoLocations = edge.asPolyLine().stream()
.map(protoLocationConverter::backwardConvert).collect(Collectors.toList());
protoEdgeBuilder.addAllShapePoints(protoLocations);

final Map<String, String> tags = edge.getTags();
protoEdgeBuilder.addAllTags(protoTagListConverter.backwardConvert(tags));

numberOfEdges++;
protoAtlasBuilder.addEdges(protoEdgeBuilder.build());
}
protoAtlasBuilder.setNumberOfEdges(numberOfEdges);
}

private void writeLinesToBuilder(final Atlas atlas,
final ProtoAtlasContainer.Builder protoAtlasBuilder)
{
Expand All @@ -206,6 +272,29 @@ private void writeLinesToBuilder(final Atlas atlas,
protoAtlasBuilder.setNumberOfLines(numberOfLines);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above


private void writeNodesToBuilder(final Atlas atlas, final Builder protoAtlasBuilder)
{
long numberOfNodes = 0;
final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();
final ProtoTagListConverter protoTagListConverter = new ProtoTagListConverter();

for (final Node node : atlas.nodes())
{
final ProtoNode.Builder protoNodeBuilder = ProtoNode.newBuilder();

protoNodeBuilder.setId(node.getIdentifier());
protoNodeBuilder
.setLocation(protoLocationConverter.backwardConvert(node.getLocation()));

final Map<String, String> tags = node.getTags();
protoNodeBuilder.addAllTags(protoTagListConverter.backwardConvert(tags));

numberOfNodes++;
protoAtlasBuilder.addNodes(protoNodeBuilder.build());
}
protoAtlasBuilder.setNumberOfNodes(numberOfNodes);
}

private void writePointsToBuilder(final Atlas atlas,
final ProtoAtlasContainer.Builder protoAtlasBuilder)
{
Expand Down
8 changes: 8 additions & 0 deletions src/main/proto/AtlasContainer.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ package org.openstreetmap.atlas.proto;
import "Point.proto";
import "Line.proto";
import "Area.proto";
import "Node.proto";
import "Edge.proto";

message ProtoAtlasContainer {
optional int64 numberOfPoints = 1;
Expand All @@ -18,4 +20,10 @@ message ProtoAtlasContainer {

optional int64 numberOfAreas = 5;
repeated ProtoArea areas = 6;

optional int64 numberOfNodes = 7;
repeated ProtoNode nodes = 8;

optional int64 numberOfEdges = 9;
repeated ProtoEdge edges = 10;
}
9 changes: 3 additions & 6 deletions src/main/proto/Edge.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,11 @@ option java_outer_classname = "ProtoEdgeWrapper";

package org.openstreetmap.atlas.proto;

import "Node.proto";
import "Location.proto";
import "Tag.proto";

message ProtoEdge {
optional int64 id = 1;

optional ProtoNode startNode = 2;
optional ProtoNode endNode = 3;

repeated ProtoTag tags = 4;
repeated ProtoLocation shapePoints = 2;
repeated ProtoTag tags = 5;
}
4 changes: 0 additions & 4 deletions src/main/proto/Node.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,5 @@ import "Tag.proto";
message ProtoNode {
optional int64 id = 1;
optional ProtoLocation location = 2;

repeated int64 inEdgeIds = 3;
repeated int64 outEdgeIds = 4;

repeated ProtoTag tags = 5;
}
17 changes: 11 additions & 6 deletions ...est/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,19 @@ private PackedAtlasBuilder setUpTestAtlasBuilder()
packedAtlasBuilder.addArea(getNextId(), new Polygon(shapePoints), tags);

// add nodes
// tags.clear();
// tags.put("sometag:namespace", "somevalue");
// packedAtlasBuilder.addNode(getNextId(), Location.forString("48.3406719,10.5563445"),
// tags);
// tags.clear();
// packedAtlasBuilder.addNode(getNextId(), Location.forString("48.34204,10.55844"), tags);
tags.clear();
tags.put("sometag:namespace", "somevalue");
packedAtlasBuilder.addNode(getNextId(), Location.forString("48.3406719,10.5563445"), tags);
tags.clear();
packedAtlasBuilder.addNode(getNextId(), Location.forString("48.34204,10.55844"), tags);

// add edges
tags.clear();
tags.put("edge", "yes");
shapePoints.clear();
shapePoints.add(Location.forString("48.3406719,10.5563445"));
shapePoints.add(Location.forString("48.34204,10.55844"));
packedAtlasBuilder.addEdge(getNextId(), new PolyLine(shapePoints), tags);

return packedAtlasBuilder;
}
Expand Down