Skip to content

Add support for geo_shape represented as Well-Known Text (WKT) #3377

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.

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

Merged
merged 7 commits into from
Aug 27, 2018

Conversation

russcam
Copy link
Contributor

@russcam russcam commented Aug 24, 2018

This PR adds support for Well-Known Text (WKT) representations
of geo_shape types. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

  • Move GeoShape Query tests into Tests project
  • Add support for Z values in GeoCoordinate
  • Deserialize from WKT to IGeoShape types. The format from which
    the shape is deserialized is assigned to an internal format property
    on the concrete implementations of GeoShape as the intention is
    not to expose this as a property on the IGeoShape interface. The format
    is assigned to the instance so that the IGeoShape instance is
    serialized to the same format if indexed again.
  • GeoWKTReader is a simple tokenizer implementation
    for the purposes of parsing only WKT concepts that are supported
    by Elasticsearch.
  • GeoShapeConverter now implements WriteJson() because the
    original format of IGeoShape now needs to be taken into account
    when serializing.
  • Add tests for roundtrip serialization of WKT.

Closes #3256

codebrain and others added 5 commits August 22, 2018 19:16
This commit moves the geo_shape queries into the Test project.
Missed in the Tests refactoring
This commit adds support for Well-Known Text (WKT) representations
of geo_shape. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

Deserialize from WKT to IGeoShape types. The format from which
the shape is deserialized is assigned to an internal format property
on the concrete implementations of GeoShape as the intention is
not to expose this as a property on the IGeoShape interface. The format
is assigned to the instance so that the IGeoShape instance is
serialized to the same format if indexed again.

GeoWKTReader is a simple tokenizer implementation
for the purposes of parsing only WKT concepts that are supported
by Elasticsearch.

GeoShapeConverter now implements WriteJson because the
original format of IGeoShape now needs to be taken into account
when serializing.

Add tests for roundtrip serialization of WKT.

Closes #3256
Copy link
Contributor

@codebrain codebrain left a comment

Choose a reason for hiding this comment

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

Looks good. I would add a test to assert a malformed WKT string.

throw new NotSupportedException();
// IGeometryCollection needs to be handled separately because it does not
// implement IGeoShape, and can't because it would be a binary breaking change.
// This is fixed in next major release.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe put something like "Fixed in 7.0" that way when we search in the code for TODOs in 7.0 we can find this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

++

var radius = shape["radius"];
return ParseCircleGeoShape(shape, serializer, radius);
case "envelope":
case GeoShapeType.Circle:
Copy link
Contributor

Choose a reason for hiding this comment

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

++

@@ -7,7 +7,7 @@
<NoWarn>$(NoWarn);xUnit1013</NoWarn>
</PropertyGroup>
<ItemGroup>
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.0-beta1-build3642" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep!

This commit adds additional unit tests for reading and
writing shapes in WKT.
@russcam russcam merged commit d61431e into 6.3 Aug 27, 2018
@Mpdreamz
Copy link
Member

Wicked!

@russcam
Copy link
Contributor Author

russcam commented Aug 27, 2018

I see what you did there @Mpdreamz 😄

@russcam russcam mentioned this pull request Aug 27, 2018
@Mpdreamz Mpdreamz mentioned this pull request Sep 3, 2018
45 tasks
Mpdreamz pushed a commit that referenced this pull request Sep 3, 2018
* Add support for well known text (wkt) to geo bounding box queries
* Move geo_shape queries into Tests project

This commit moves the geo_shape queries into the Test project.
Missed in the Tests refactoring

* Add support for Z values to GeoCoordinate
* Add support for WKT geo shapes

This commit adds support for Well-Known Text (WKT) representations
of geo_shape. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

Deserialize from WKT to IGeoShape types. The format from which
the shape is deserialized is assigned to an internal format property
on the concrete implementations of GeoShape as the intention is
not to expose this as a property on the IGeoShape interface. The format
is assigned to the instance so that the IGeoShape instance is
serialized to the same format if indexed again.

GeoWKTReader is a simple tokenizer implementation
for the purposes of parsing only WKT concepts that are supported
by Elasticsearch.

GeoShapeConverter now implements WriteJson because the
original format of IGeoShape now needs to be taken into account
when serializing.

Add tests for roundtrip serialization of WKT.

Closes #3256
Mpdreamz pushed a commit that referenced this pull request Sep 3, 2018
* Add support for well known text (wkt) to geo bounding box queries
* Move geo_shape queries into Tests project

This commit moves the geo_shape queries into the Test project.
Missed in the Tests refactoring

* Add support for Z values to GeoCoordinate
* Add support for WKT geo shapes

This commit adds support for Well-Known Text (WKT) representations
of geo_shape. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

Deserialize from WKT to IGeoShape types. The format from which
the shape is deserialized is assigned to an internal format property
on the concrete implementations of GeoShape as the intention is
not to expose this as a property on the IGeoShape interface. The format
is assigned to the instance so that the IGeoShape instance is
serialized to the same format if indexed again.

GeoWKTReader is a simple tokenizer implementation
for the purposes of parsing only WKT concepts that are supported
by Elasticsearch.

GeoShapeConverter now implements WriteJson because the
original format of IGeoShape now needs to be taken into account
when serializing.

Add tests for roundtrip serialization of WKT.

Closes #3256
russcam added a commit that referenced this pull request Sep 12, 2018
* Add support for well known text (wkt) to geo bounding box queries
* Move geo_shape queries into Tests project

This commit moves the geo_shape queries into the Test project.
Missed in the Tests refactoring

* Add support for Z values to GeoCoordinate
* Add support for WKT geo shapes

This commit adds support for Well-Known Text (WKT) representations
of geo_shape. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

Deserialize from WKT to IGeoShape types. The format from which
the shape is deserialized is assigned to an internal format property
on the concrete implementations of GeoShape as the intention is
not to expose this as a property on the IGeoShape interface. The format
is assigned to the instance so that the IGeoShape instance is
serialized to the same format if indexed again.

GeoWKTReader is a simple tokenizer implementation
for the purposes of parsing only WKT concepts that are supported
by Elasticsearch.

GeoShapeConverter now implements WriteJson because the
original format of IGeoShape now needs to be taken into account
when serializing.

Add tests for roundtrip serialization of WKT.

Closes #3256

(cherry picked from commit f31b087)

Includes line ending sensitive unit test from 0d0afdb
russcam added a commit that referenced this pull request Sep 12, 2018
* Add support for well known text (wkt) to geo bounding box queries
* Move geo_shape queries into Tests project

This commit moves the geo_shape queries into the Test project.
Missed in the Tests refactoring

* Add support for Z values to GeoCoordinate
* Add support for WKT geo shapes

This commit adds support for Well-Known Text (WKT) representations
of geo_shape. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

Deserialize from WKT to IGeoShape types. The format from which
the shape is deserialized is assigned to an internal format property
on the concrete implementations of GeoShape as the intention is
not to expose this as a property on the IGeoShape interface. The format
is assigned to the instance so that the IGeoShape instance is
serialized to the same format if indexed again.

GeoWKTReader is a simple tokenizer implementation
for the purposes of parsing only WKT concepts that are supported
by Elasticsearch.

GeoShapeConverter now implements WriteJson because the
original format of IGeoShape now needs to be taken into account
when serializing.

Add tests for roundtrip serialization of WKT.

Closes #3256

(cherry picked from commit f31b087)

Includes line ending sensitive unit test from 0d0afdb
Mpdreamz pushed a commit that referenced this pull request Sep 14, 2018
#3401)

* Add support for well known text (wkt) to geo bounding box queries
* Move geo_shape queries into Tests project

This commit moves the geo_shape queries into the Test project.
Missed in the Tests refactoring

* Add support for Z values to GeoCoordinate
* Add support for WKT geo shapes

This commit adds support for Well-Known Text (WKT) representations
of geo_shape. The extent of the implementation is only as far as is
required by the WKT support in Elasticsearch.

Deserialize from WKT to IGeoShape types. The format from which
the shape is deserialized is assigned to an internal format property
on the concrete implementations of GeoShape as the intention is
not to expose this as a property on the IGeoShape interface. The format
is assigned to the instance so that the IGeoShape instance is
serialized to the same format if indexed again.

GeoWKTReader is a simple tokenizer implementation
for the purposes of parsing only WKT concepts that are supported
by Elasticsearch.

GeoShapeConverter now implements WriteJson because the
original format of IGeoShape now needs to be taken into account
when serializing.

Add tests for roundtrip serialization of WKT.

Closes #3256

(cherry picked from commit f31b087)

Includes line ending sensitive unit test from 0d0afdb
@russcam russcam deleted the feature/wkt branch October 18, 2018 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants