From fe7bbf24c5720a6f39d658fa0f8852a479704f1f Mon Sep 17 00:00:00 2001 From: Johnathan Garrett Date: Tue, 14 Feb 2017 04:35:24 -0500 Subject: [PATCH] Add ability to specify bounds for spatial indexes. Signed-off-by: Johnathan Garrett --- doc/manpages/source/indexcreate.rst | 2 + doc/manpages/source/indexupdate.rst | 2 + .../source/interaction/indexing-web-api.rst | 48 ++++++- .../cli/porcelain/index/CreateIndex.java | 8 ++ .../cli/porcelain/index/UpdateIndex.java | 10 +- .../functional/DefaultStepDefinitions.java | 19 +++ .../features/index/CreateIndex.feature | 51 ++++++- .../features/index/UpdateIndex.feature | 131 +++++++++++++----- .../porcelain/index/CreateQuadTree.java | 16 ++- .../geogig/porcelain/index/UpdateIndexOp.java | 87 ++++++++---- .../geogig/repository/impl/SpatialOps.java | 37 +++++ .../porcelain/index/UpdateIndexOpTest.java | 66 +++++++-- .../geogig/web/api/index/CreateIndex.java | 14 ++ .../geogig/web/api/index/UpdateIndex.java | 14 ++ .../geogig/web/api/index/CreateIndexTest.java | 56 +++++++- .../geogig/web/api/index/UpdateIndexTest.java | 79 +++++++++++ .../web/functional/WebAPICucumberHooks.java | 20 +++ .../features/commands/IndexCreate.feature | 22 ++- .../features/commands/IndexUpdate.feature | 51 +++++-- 19 files changed, 637 insertions(+), 96 deletions(-) diff --git a/doc/manpages/source/indexcreate.rst b/doc/manpages/source/indexcreate.rst index be6daa4933..e50cf5d266 100644 --- a/doc/manpages/source/indexcreate.rst +++ b/doc/manpages/source/indexcreate.rst @@ -25,6 +25,8 @@ OPTIONS --index-history If specified, indexes will be created for all commits in the history. +--bounds If specified, the max bounds of the spatial index will be set to this parameter. + SEE ALSO diff --git a/doc/manpages/source/indexupdate.rst b/doc/manpages/source/indexupdate.rst index ff6b2afca3..d99a7591d6 100644 --- a/doc/manpages/source/indexupdate.rst +++ b/doc/manpages/source/indexupdate.rst @@ -29,6 +29,8 @@ OPTIONS --overwrite If specified, new attributes will replace the existing set of extra attributes. +--bounds If specified, the max bounds of the spatial index will be updated to this parameter. + SEE ALSO diff --git a/doc/manual/source/interaction/indexing-web-api.rst b/doc/manual/source/interaction/indexing-web-api.rst index 9553cd6d1a..c83dea39a0 100644 --- a/doc/manual/source/interaction/indexing-web-api.rst +++ b/doc/manual/source/interaction/indexing-web-api.rst @@ -10,7 +10,7 @@ Creates a new index on a specified feature tree using a geometry attribute in th :: - PUT /repos//index/create[.xml|.json]?treeRefSpec=[&geometryAttributeName=][[&extraAttributes=]+][&indexHistory=] + PUT /repos//index/create[.xml|.json]?treeRefSpec=[&geometryAttributeName=][[&extraAttributes=]+][&indexHistory=][&bounds=] Parameters @@ -28,6 +28,9 @@ Optional. An extra attribute that should be stored in the index to improve perfo **indexHistory:** Optional. Boolean indicating whether or not index trees should be built for every commit in the history of the repository. By default only the feature tree in commit indicated by the ``treeRefSpec`` will be indexed. +**bounds:** +Optional. String indicating the max bounds of the spatial index. If not specified, the bounds will be set to the extent of the coordinate reference system of the geometry attribute. + Examples ^^^^^^^^ @@ -72,6 +75,26 @@ Create an index with extra attributes: b3340540d2098ec33b7edab1b38d3ffc18f8e162 + +Create an index with custom bounds: +*********************************** + +:: + + $ curl -X PUT -v "http://localhost:8182/repos/repo1/index/create?treeRefSpec=Points&bounds=-60,-45,60,45" | xmllint --format - + < HTTP/1.1 201 Created + < Content-Type: application/xml + + + true + + Points + the_geom + QUADTREE + Env[-60,60,-45,45] + + b3340540d2098ec33b7edab1b38d3ffc18f8e162 + Index Update @@ -105,6 +128,9 @@ Optional. If extra attributes already exist on the index, you must specify eithe **overwrite:** Optional: See ``add``. If ``overwrite`` is specified, the extra attributes in the indexed will be replaced with those specified in the parameters. If no extra attributes are supplied, all extra attributes will be removed from the index. +**bounds:** +Optional. String indicating the new max bounds of the spatial index. + Examples ^^^^^^^^ @@ -176,6 +202,26 @@ In this case Points already has an extra attribute of ``sp``. If we want to rem b3340540d2098ec33b7edab1b38d3ffc18f8e162 + +Update the max bounds of the Points index: +****************************************** + +:: + + $ curl -X POST -v "http://localhost:8182/repos/repo1/index/update?treeRefSpec=Points&bounds=-60,-45,60,45" | xmllint --format - + < HTTP/1.1 201 Created + < Content-Type: application/xml + + + true + + Points + the_geom + QUADTREE + Env[-60,60,-45,45] + + b3340540d2098ec33b7edab1b38d3ffc18f8e162 + Index Rebuild diff --git a/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/CreateIndex.java b/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/CreateIndex.java index 1a829de2df..2542088d2d 100644 --- a/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/CreateIndex.java +++ b/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/CreateIndex.java @@ -21,9 +21,11 @@ import org.locationtech.geogig.porcelain.index.CreateQuadTree; import org.locationtech.geogig.porcelain.index.Index; import org.locationtech.geogig.repository.Repository; +import org.locationtech.geogig.repository.impl.SpatialOps; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; +import com.vividsolutions.jts.geom.Envelope; @RequiresRepository(true) @Parameters(commandNames = { @@ -44,17 +46,23 @@ public class CreateIndex extends AbstractCommand implements CLICommand { "--extra-attributes" }, description = "Comma separated list of extra attribute names to hold inside index") private List extraAttributes; + @Parameter(names = "--bounds", description = "If specified, the max bounds of the spatial index will be set to this parameter. ") + private String bbox; + @Override protected void runInternal(GeogigCLI cli) throws InvalidParameterException, CommandFailedException, IOException { Repository repo = cli.getGeogig().getRepository(); + Envelope envelope = SpatialOps.parseNonReferencedBBOX(bbox); + Index index = repo.command(CreateQuadTree.class)// .setTreeRefSpec(treeRefSpec)// .setGeometryAttributeName(attribute)// .setExtraAttributes(extraAttributes)// .setIndexHistory(indexHistory)// + .setBounds(envelope)// .setProgressListener(cli.getProgressListener())// .call(); diff --git a/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/UpdateIndex.java b/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/UpdateIndex.java index d3065b4169..47cd919351 100644 --- a/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/UpdateIndex.java +++ b/src/cli/src/main/java/org/locationtech/geogig/cli/porcelain/index/UpdateIndex.java @@ -21,9 +21,11 @@ import org.locationtech.geogig.porcelain.index.Index; import org.locationtech.geogig.porcelain.index.UpdateIndexOp; import org.locationtech.geogig.repository.Repository; +import org.locationtech.geogig.repository.impl.SpatialOps; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; +import com.vividsolutions.jts.geom.Envelope; @RequiresRepository(true) @Parameters(commandNames = { @@ -49,6 +51,9 @@ public class UpdateIndex extends AbstractCommand implements CLICommand { "--add" }, description = "Add new attributes to existing list of extra attributes held by the index") private boolean add; + @Parameter(names = "--bounds", description = "If specified, the max bounds of the spatial index will be updated to this parameter. ") + private String bbox; + @Parameter(names = "--index-history", description = "If specified, indexes will be rebuilt for all commits in the history.") private boolean indexHistory = false; @@ -58,13 +63,16 @@ protected void runInternal(GeogigCLI cli) Repository repo = cli.getGeogig().getRepository(); + Envelope envelope = SpatialOps.parseNonReferencedBBOX(bbox); + Index index = repo.command(UpdateIndexOp.class)// - .setTreeRefSpec(treeRefSpec) + .setTreeRefSpec(treeRefSpec)// .setAttributeName(attribute)// .setExtraAttributes(extraAttributes)// .setOverwrite(overwrite)// .setAdd(add)// .setIndexHistory(indexHistory)// + .setBounds(envelope)// .setProgressListener(cli.getProgressListener())// .call(); diff --git a/src/cli/src/test/java/org/locationtech/geogig/cli/test/functional/DefaultStepDefinitions.java b/src/cli/src/test/java/org/locationtech/geogig/cli/test/functional/DefaultStepDefinitions.java index db97b95c92..6fb0efb0d5 100644 --- a/src/cli/src/test/java/org/locationtech/geogig/cli/test/functional/DefaultStepDefinitions.java +++ b/src/cli/src/test/java/org/locationtech/geogig/cli/test/functional/DefaultStepDefinitions.java @@ -71,6 +71,7 @@ import org.locationtech.geogig.repository.RepositoryResolver; import org.locationtech.geogig.repository.WorkingTree; import org.locationtech.geogig.repository.impl.GeoGIG; +import org.locationtech.geogig.repository.impl.SpatialOps; import org.opengis.feature.Feature; import org.opengis.feature.type.PropertyDescriptor; import org.slf4j.Logger; @@ -83,6 +84,7 @@ import com.google.common.collect.Maps; import com.google.common.collect.SetMultimap; import com.google.common.io.Files; +import com.vividsolutions.jts.geom.Envelope; import cucumber.api.DataTable; import cucumber.api.Scenario; @@ -834,6 +836,23 @@ public void noIndexAtCommit(String headRef) throws Throwable { assertFalse(indexTreeId.isPresent()); } + @Then("^the repository's \"([^\"]*)\" index bounds should be \"([^\"]*)\"$") + public void verifyIndexBounds(String headRef, String bbox) throws Throwable { + Repository repo = localRepo.geogigCLI.getGeogig().getRepository(); + final NodeRef typeTreeRef = IndexUtils.resolveTypeTreeRef(repo.context(), headRef); + String treeName = typeTreeRef.path(); + IndexInfo indexInfo = IndexUtils.resolveIndexInfo(repo.indexDatabase(), treeName, null); + Map metadata = indexInfo.getMetadata(); + assertTrue(metadata.containsKey(IndexInfo.MD_QUAD_MAX_BOUNDS)); + Envelope indexBounds = (Envelope) metadata.get(IndexInfo.MD_QUAD_MAX_BOUNDS); + Envelope expected = SpatialOps.parseNonReferencedBBOX(bbox); + final double EPSILON = 0.00001; + assertEquals(expected.getMinX(), indexBounds.getMinX(), EPSILON); + assertEquals(expected.getMaxX(), indexBounds.getMaxX(), EPSILON); + assertEquals(expected.getMinY(), indexBounds.getMinY(), EPSILON); + assertEquals(expected.getMaxY(), indexBounds.getMaxY(), EPSILON); + } + @SuppressWarnings("unchecked") @Then("^the repository's \"([^\"]*)\" index should track the extra attribute \"([^\"]*)\"$") public void verifyIndexExtraAttributes(String headRef, String attributeName) throws Throwable { diff --git a/src/cli/src/test/resources/features/index/CreateIndex.feature b/src/cli/src/test/resources/features/index/CreateIndex.feature index de955d5b79..1019797b99 100644 --- a/src/cli/src/test/resources/features/index/CreateIndex.feature +++ b/src/cli/src/test/resources/features/index/CreateIndex.feature @@ -6,11 +6,12 @@ Feature: "index create" command Scenario: Try to create an index Given I have a repository And I have several commits - And I run the command "index create --tree Points" + When I run the command "index create --tree Points" Then the response should contain "Index created successfully" And the response should contain "Size: 3" And the response should not contain "Size: 2" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -25,11 +26,12 @@ Feature: "index create" command Scenario: Try to create an index with extra attributes Given I have a repository And I have several commits - And I run the command "index create --tree Points --extra-attributes sp" + When I run the command "index create --tree Points --extra-attributes sp" Then the response should contain "Index created successfully" And the response should contain "Size: 3" And the response should not contain "Size: 2" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -40,33 +42,66 @@ Feature: "index create" command And the repository's "HEAD~1:Points" should not have an index And the repository's "HEAD~2:Points" should not have an index And the repository's "HEAD~3:Points" should not have an index + + Scenario: Try to create an index with user-specified bounds + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --bounds -45,-45,45,45" + Then the response should contain "Index created successfully" + And the response should contain "Size: 3" + And the response should not contain "Size: 2" + And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-45,-45,45,45" + And the repository's "HEAD:Points" index should not track the extra attribute "sp" + And the repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repository's "HEAD:Points" index should have the following features: + | index | + | Points.1 | + | Points.2 | + | Points.3 | + And the repository's "HEAD~1:Points" should not have an index + And the repository's "HEAD~2:Points" should not have an index + And the repository's "HEAD~3:Points" should not have an index + + Scenario: Try to create an index with too few bounds parameters + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --bounds -45,-45,45" + Then the response should contain "Invalid bbox parameter: '-45,-45,45'. Expected format: " + + Scenario: Try to create an index with invalid bounds parameters + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --bounds -45,-45,45,A" + Then the response should contain "Invalid bbox parameter: '-45,-45,45,A'. Expected format: " Scenario: Try to create an index on a nonexistent tree Given I have a repository And I have several commits - And I run the command "index create --tree nonexistent" + When I run the command "index create --tree nonexistent" Then the response should contain "Can't find feature tree 'nonexistent'" Scenario: Try to create an index on a nonexistent attribute Given I have a repository And I have several commits - And I run the command "index create --tree Points --attribute nonexistent" + When I run the command "index create --tree Points --attribute nonexistent" Then the response should contain "property nonexistent does not exist" Scenario: Try to create an index on a non-geometry attribute Given I have a repository And I have several commits - And I run the command "index create --tree Points --attribute sp" + When I run the command "index create --tree Points --attribute sp" Then the response should contain "property sp is not a geometry attribute" Scenario: Try to create an index with the full history Given I have a repository And I have several branches - And I run the command "index create --tree Points --index-history" + When I run the command "index create --tree Points --index-history" Then the response should contain "Index created successfully" And the response should contain "Size: 3" And the response should contain "Size: 2" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -95,11 +130,12 @@ Feature: "index create" command Scenario: Try to create an index with the full history and extra attributes Given I have a repository And I have several branches - And I run the command "index create --tree Points --extra-attributes sp,ip --index-history" + When I run the command "index create --tree Points --extra-attributes sp,ip --index-history" Then the response should contain "Index created successfully" And the response should contain "Size: 3" And the response should contain "Size: 2" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should track the extra attribute "sp" And the repository's "HEAD:Points" index should track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -144,6 +180,7 @@ Feature: "index create" command Then the response should contain "Index updated" And the response should contain "Size: 1" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: diff --git a/src/cli/src/test/resources/features/index/UpdateIndex.feature b/src/cli/src/test/resources/features/index/UpdateIndex.feature index 9d44ee2455..8e802ed80b 100644 --- a/src/cli/src/test/resources/features/index/UpdateIndex.feature +++ b/src/cli/src/test/resources/features/index/UpdateIndex.feature @@ -6,8 +6,9 @@ Feature: "index update" command Scenario: Try to update an index Given I have a repository And I have several commits - And I run the command "index create --tree Points" + When I run the command "index create --tree Points" Then the response should contain "Index created successfully" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -19,6 +20,7 @@ Feature: "index update" command Then the response should contain "Index updated successfully" And the response should contain "Size: 3" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -30,14 +32,15 @@ Feature: "index update" command Scenario: Try to update a nonexistent index Given I have a repository And I have several commits - And I run the command "index update --tree nonexistent" + When I run the command "index update --tree nonexistent" Then the response should contain "Can't find feature tree 'nonexistent'" Scenario: Try to update an index with the full history Given I have a repository And I have several branches - And I run the command "index create --tree Points" + When I run the command "index create --tree Points" Then the response should contain "Index created successfully" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -50,6 +53,7 @@ Feature: "index update" command And the response should contain "Size: 3" And the response should contain "Size: 2" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -78,8 +82,9 @@ Feature: "index update" command Scenario: Try to add attributes to an index Given I have a repository And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" + When I run the command "index create --tree Points --extra-attributes ip" Then the response should contain "Index created successfully" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -91,6 +96,7 @@ Feature: "index update" command Then the response should contain "Index updated successfully" And the response should contain "Size: 3" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should track the extra attribute "sp" And the repository's "HEAD:Points" index should track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -102,8 +108,9 @@ Feature: "index update" command Scenario: Try to replace attributes of an index Given I have a repository And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" + When I run the command "index create --tree Points --extra-attributes ip" Then the response should contain "Index created successfully" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -115,6 +122,7 @@ Feature: "index update" command Then the response should contain "Index updated successfully" And the response should contain "Size: 3" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: @@ -122,51 +130,93 @@ Feature: "index update" command | Points.1 | | Points.2 | | Points.3 | + + Scenario: Try to update the bounds of an index + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --extra-attributes sp --bounds -45,-45,45,45" + Then the response should contain "Index created successfully" + And the repository's "HEAD:Points" index bounds should be "-45,-45,45,45" + And the repository's "HEAD:Points" index should track the extra attribute "sp" + And the repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repository's "HEAD:Points" index should have the following features: + | index | + | Points.1 | + | Points.2 | + | Points.3 | + When I run the command "index update --tree Points --bounds -20,-45,20,45" + Then the response should contain "Index updated successfully" + And the response should contain "Size: 3" + And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-20,-45,20,45" + And the repository's "HEAD:Points" index should track the extra attribute "sp" + And the repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repository's "HEAD:Points" index should have the following features: + | index | + | Points.1 | + | Points.2 | + | Points.3 | + + Scenario: Try to update the bounds of an index with incorrect bounds parameter + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --extra-attributes sp --bounds -45,-45,45,45" + Then the response should contain "Index created successfully" + And the repository's "HEAD:Points" index bounds should be "-45,-45,45,45" + And the repository's "HEAD:Points" index should track the extra attribute "sp" + And the repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repository's "HEAD:Points" index should have the following features: + | index | + | Points.1 | + | Points.2 | + | Points.3 | + When I run the command "index update --tree Points --bounds -20,-45,20" + Then the response should contain "Invalid bbox parameter: '-20,-45,20'. Expected format: " Scenario: Try to change existing attributes without specifying add or overwrite Given I have a repository And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" + When I run the command "index create --tree Points --extra-attributes ip" Then the response should contain "Index created successfully" When I run the command "index update --tree Points --extra-attributes sp" Then the response should contain "Extra attributes already exist on index, specify add or overwrite to update." Scenario: I try to update the index without specifying a tree Given I have a repository - And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" - Then the response should contain "Index created successfully" - When I run the command "index update --extra-attributes sp" - Then the response should contain "The following option is required: --tree" + And I have several commits + When I run the command "index create --tree Points --extra-attributes ip" + Then the response should contain "Index created successfully" + When I run the command "index update --extra-attributes sp" + Then the response should contain "The following option is required: --tree" Scenario: I try to update the index for a non-existent attribute Given I have a repository - And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" - Then the response should contain "Index created successfully" - When I run the command "index update --tree Points --attribute fakeAttrib" - Then the response should contain "A matching index could not be found." + And I have several commits + When I run the command "index create --tree Points --extra-attributes ip" + Then the response should contain "Index created successfully" + When I run the command "index update --tree Points --attribute fakeAttrib" + Then the response should contain "A matching index could not be found." Scenario: I try to update the index leaving the extra-attribute param empty Given I have a repository - And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" - Then the response should contain "Index created successfully" - When I run the command "index update --tree Points --extra-attributes" - Then the response should contain "Expected a value after parameter --extra-attributes" + And I have several commits + When I run the command "index create --tree Points --extra-attributes ip" + Then the response should contain "Index created successfully" + When I run the command "index update --tree Points --extra-attributes" + Then the response should contain "Expected a value after parameter --extra-attributes" Scenario: I try to update the index for a non-existent extra-attribute Given I have a repository - And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" - Then the response should contain "Index created successfully" - When I run the command "index update --tree Points --extra-attributes fakeAttrib" - Then the response should contain "FeatureType Points does not define attribute 'fakeAttrib'" + And I have several commits + When I run the command "index create --tree Points --extra-attributes ip" + Then the response should contain "Index created successfully" + When I run the command "index update --tree Points --extra-attributes fakeAttrib" + Then the response should contain "FeatureType Points does not define attribute 'fakeAttrib'" Scenario: I try to overwrite a non-existent extra-attribute Given I have a repository And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" + When I run the command "index create --tree Points --extra-attributes ip" Then the response should contain "Index created successfully" When I run the command "index update --tree Points --extra-attributes fakeAttrib --overwrite" Then the response should contain "FeatureType Points does not define attribute 'fakeAttrib'" @@ -175,25 +225,42 @@ Feature: "index update" command Given I have a repository And I have staged "points1" And I run the command "commit -m "point1 added" - And I run the command "index create --tree Points --extra-attributes ip" + When I run the command "index create --tree Points --extra-attributes ip" Then the response should contain "Index created successfully" When I run the command "index update --tree Points --index-history --overwrite" Then the response should contain "Index updated successfully" And the response should contain "Size: 1" And the response should contain the index ID for tree "Points" + And the repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repository's "HEAD:Points" index should not track the extra attribute "sp" And the repository's "HEAD:Points" index should not track the extra attribute "ip" And the repository's "HEAD:Points" index should have the following features: | index | | Points.1 | - Scenario: I try to update the index for the full history without specifying overwrite + Scenario: I try to update the index without updating anything + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --extra-attributes ip" + Then the response should contain "Index created successfully" + When I run the command "index update --tree Points" + Then the response should contain "Nothing to update..." + + Scenario: I try to update the index with the same extra attribute + Given I have a repository + And I have several commits + When I run the command "index create --tree Points --extra-attributes ip" + Then the response should contain "Index created successfully" + When I run the command "index update --tree Points --extra-attributes ip --overwrite" + Then the response should contain "Nothing to update..." + + Scenario: I try to update the index by adding the same extra attribute Given I have a repository And I have several commits - And I run the command "index create --tree Points --extra-attributes ip" + When I run the command "index create --tree Points --extra-attributes sp,ip" Then the response should contain "Index created successfully" - When I run the command "index update --tree Points --index-history" - Then the response should contain "Extra attributes already exist on index, specify add or overwrite" + When I run the command "index update --tree Points --extra-attributes ip --add" + Then the response should contain "Nothing to update..." Scenario: I try to update the index in an empty repository Given I have a repository diff --git a/src/core/src/main/java/org/locationtech/geogig/porcelain/index/CreateQuadTree.java b/src/core/src/main/java/org/locationtech/geogig/porcelain/index/CreateQuadTree.java index 55021693c8..ac83255c64 100644 --- a/src/core/src/main/java/org/locationtech/geogig/porcelain/index/CreateQuadTree.java +++ b/src/core/src/main/java/org/locationtech/geogig/porcelain/index/CreateQuadTree.java @@ -47,6 +47,8 @@ public class CreateQuadTree extends AbstractGeoGigOp { private @Nullable String geometryAttributeName; + private @Nullable Envelope bounds; + /** * @param typeTreeRef the {@link NodeRef} of the canonical tree to build a quadtree from * @return {@code this} @@ -97,6 +99,17 @@ public CreateQuadTree setIndexHistory(boolean indexHistory) { return this; } + /** + * Sets the bounds of the quad tree. + * + * @param bounds the {@link Envelope} that represents the bounds of the quad tree + * @return {@code this} + */ + public CreateQuadTree setBounds(Envelope bounds) { + this.bounds = bounds; + return this; + } + /** * Performs the operation. * @@ -115,7 +128,8 @@ protected Index _call() { final GeometryDescriptor geometryAtt = IndexUtils .resolveGeometryAttribute(featureType, geometryAttributeName); - final Envelope maxBounds = IndexUtils.resolveMaxBounds(geometryAtt); + final Envelope maxBounds = this.bounds != null ? this.bounds + : IndexUtils.resolveMaxBounds(geometryAtt); final @Nullable String[] extraAttributes = IndexUtils .resolveMaterializedAttributeNames(featureType, this.extraAttributes); diff --git a/src/core/src/main/java/org/locationtech/geogig/porcelain/index/UpdateIndexOp.java b/src/core/src/main/java/org/locationtech/geogig/porcelain/index/UpdateIndexOp.java index 090b114574..b0da35d9e2 100644 --- a/src/core/src/main/java/org/locationtech/geogig/porcelain/index/UpdateIndexOp.java +++ b/src/core/src/main/java/org/locationtech/geogig/porcelain/index/UpdateIndexOp.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; +import java.util.Set; import org.eclipse.jdt.annotation.Nullable; import org.locationtech.geogig.model.ObjectId; @@ -25,8 +26,9 @@ import org.locationtech.geogig.repository.NodeRef; import com.google.common.base.Optional; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import com.vividsolutions.jts.geom.Envelope; /** * Updates an {@link IndexInfo} with new metadata. @@ -45,6 +47,8 @@ public class UpdateIndexOp extends AbstractGeoGigOp { private boolean indexHistory = false; + private Envelope bounds = null; + /** * @param treeRefSpec the tree refspec of the index to be updated * @return {@code this} @@ -108,6 +112,17 @@ public UpdateIndexOp setIndexHistory(boolean indexHistory) { return this; } + /** + * Sets the bounds of the spatial index. + * + * @param bounds the {@link Envelope} that represents the bounds of the spatial index + * @return {@code this} + */ + public UpdateIndexOp setBounds(Envelope bounds) { + this.bounds = bounds; + return this; + } + /** * Performs the operation. * @@ -130,39 +145,45 @@ protected Index _call() { Map newMetadata = Maps.newHashMap(oldIndexInfo.getMetadata()); String[] oldAttributes = (String[]) newMetadata .get(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA); - List updatedAttributes; - if (oldAttributes == null || oldAttributes.length == 0) { - if (newAttributes == null) { - updatedAttributes = null; + String[] updatedAttributes; + if (add) { + if (oldAttributes == null) { + updatedAttributes = newAttributes; + } else if (newAttributes == null) { + updatedAttributes = oldAttributes; } else { - updatedAttributes = Lists.newArrayList(newAttributes); + Set oldSet = Sets.newHashSet(oldAttributes); + Set newSet = Sets.newHashSet(newAttributes); + oldSet.addAll(newSet); + updatedAttributes = oldSet.toArray(new String[oldSet.size()]); } - } else { - checkState(overwrite || add, + } else if (overwrite) { + updatedAttributes = newAttributes; + } else if (newAttributes != null) { + checkState(oldAttributes == null, "Extra attributes already exist on index, specify add or overwrite to update."); - if (overwrite) { - if (newAttributes == null) { - updatedAttributes = null; - } else { - updatedAttributes = Lists.newArrayList(newAttributes); - } + updatedAttributes = newAttributes; + } else { + updatedAttributes = oldAttributes; + } + + boolean updated = false; + if (!contentsEqual(updatedAttributes, oldAttributes)) { + if (updatedAttributes == null) { + newMetadata.remove(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA); } else { - updatedAttributes = Lists.newArrayList(oldAttributes); - if (newAttributes != null) { - for (int i = 0; i < newAttributes.length; i++) { - if (!updatedAttributes.contains(newAttributes[i])) { - updatedAttributes.add(newAttributes[i]); - } - } - } + newMetadata.put(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA, updatedAttributes); } + updated = true; } - if (updatedAttributes == null) { - newMetadata.remove(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA); - } else { - newMetadata.put(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA, - updatedAttributes.toArray(new String[updatedAttributes.size()])); + + if (bounds != null) { + newMetadata.put(IndexInfo.MD_QUAD_MAX_BOUNDS, bounds); + updated = true; } + + checkState(updated, "Nothing to update..."); + newIndexInfo = indexDatabase().updateIndexInfo(treeName, oldIndexInfo.getAttributeName(), oldIndexInfo.getIndexType(), newMetadata); @@ -193,4 +214,16 @@ protected Index _call() { return new Index(newIndexInfo, indexedTreeId, indexDatabase()); } + + private boolean contentsEqual(@Nullable String[] left, @Nullable String[] right) { + if (left == right) { + return true; + } + if (left == null || right == null) { + return false; + } + Set leftSet = Sets.newHashSet(left); + Set rightSet = Sets.newHashSet(right); + return leftSet.containsAll(rightSet) && rightSet.containsAll(leftSet); + } } diff --git a/src/core/src/main/java/org/locationtech/geogig/repository/impl/SpatialOps.java b/src/core/src/main/java/org/locationtech/geogig/repository/impl/SpatialOps.java index d782dcbc7b..d529c317a6 100644 --- a/src/core/src/main/java/org/locationtech/geogig/repository/impl/SpatialOps.java +++ b/src/core/src/main/java/org/locationtech/geogig/repository/impl/SpatialOps.java @@ -157,6 +157,43 @@ public static ReferencedEnvelope parseBBOX(final @Nullable String bboxArg) { return env; } + /** + * Parses a bounding box in the format {@code } + *

+ * The oridinates must be given in "longitude first" format + * + * @throws IllegalArgumentException if the argument doesn't match the expected format + */ + @Nullable + public static Envelope parseNonReferencedBBOX(final @Nullable String bboxArg) { + if (bboxArg == null) { + return null; + } + List split = Splitter.on(',').omitEmptyStrings().splitToList(bboxArg); + if (split.size() != 4) { + throw new IllegalArgumentException(String.format( + "Invalid bbox parameter: '%s'. Expected format: ", + bboxArg)); + } + double minx; + double miny; + double maxx; + double maxy; + try { + minx = Double.parseDouble(split.get(0)); + miny = Double.parseDouble(split.get(1)); + maxx = Double.parseDouble(split.get(2)); + maxy = Double.parseDouble(split.get(3)); + } catch (NumberFormatException nfe) { + throw new IllegalArgumentException(String.format( + "Invalid bbox parameter: '%s'. Expected format: ", + bboxArg)); + } + + Envelope env = new Envelope(minx, maxx, miny, maxy); + return env; + } + public static CoordinateReferenceSystem findIdentifier(GeometryDescriptor geometryDescriptor) throws FactoryException, CRSException { CoordinateReferenceSystem crs = null; if (geometryDescriptor != null) { diff --git a/src/core/src/test/java/org/locationtech/geogig/porcelain/index/UpdateIndexOpTest.java b/src/core/src/test/java/org/locationtech/geogig/porcelain/index/UpdateIndexOpTest.java index 0c6082c1d3..b42b180da8 100644 --- a/src/core/src/test/java/org/locationtech/geogig/porcelain/index/UpdateIndexOpTest.java +++ b/src/core/src/test/java/org/locationtech/geogig/porcelain/index/UpdateIndexOpTest.java @@ -197,12 +197,13 @@ public void testUpdateIndexOverwriteExistingAttributes() { } @Test - public void testUpdateIndexNoAttributes() { - createIndex(); + public void testUpdateIndexRemoveExistingAttributes() { + createIndex("x"); Index index = geogig.command(UpdateIndexOp.class)// .setTreeRefSpec(worldPointsLayer.getName())// .setExtraAttributes(null)// + .setOverwrite(true)// .call(); IndexInfo indexInfo = indexdb.getIndexInfo(worldPointsLayer.getName(), "geom").get(); @@ -226,13 +227,16 @@ public void testUpdateIndexNoAttributes() { } @Test - public void testUpdateIndexRemoveExistingAttributes() { - createIndex("x"); + public void testUpdateIndexBounds() { + IndexInfo info = createIndex("x"); + Envelope oldBounds = (Envelope) info.getMetadata().get(IndexInfo.MD_QUAD_MAX_BOUNDS); + assertEquals(new Envelope(-180, 180, -90, 90), oldBounds); + + Envelope newBounds = new Envelope(-60, 60, -45, 45); Index index = geogig.command(UpdateIndexOp.class)// .setTreeRefSpec(worldPointsLayer.getName())// - .setExtraAttributes(null)// - .setOverwrite(true)// + .setBounds(newBounds)// .call(); IndexInfo indexInfo = indexdb.getIndexInfo(worldPointsLayer.getName(), "geom").get(); @@ -240,11 +244,15 @@ public void testUpdateIndexRemoveExistingAttributes() { assertEquals(worldPointsLayer.getName(), indexInfo.getTreeName()); assertEquals("geom", indexInfo.getAttributeName()); assertEquals(IndexType.QUADTREE, indexInfo.getIndexType()); - assertEquals(1, indexInfo.getMetadata().size()); + assertEquals(2, indexInfo.getMetadata().size()); assertTrue(indexInfo.getMetadata().containsKey(IndexInfo.MD_QUAD_MAX_BOUNDS)); - assertEquals(new Envelope(-180, 180, -90, 90), + assertEquals(newBounds, indexInfo.getMetadata().get(IndexInfo.MD_QUAD_MAX_BOUNDS)); - assertFalse(indexInfo.getMetadata().containsKey(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA)); + assertTrue(indexInfo.getMetadata().containsKey(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA)); + List extraAttributes = Lists.newArrayList( + (String[]) indexInfo.getMetadata().get(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA)); + assertEquals(1, extraAttributes.size()); + assertTrue(extraAttributes.contains("x")); ObjectId canonicalFeatureTreeId = geogig.command(ResolveTreeish.class) .setTreeish("HEAD:" + worldPointsLayer.getName()).call().get(); @@ -268,6 +276,43 @@ public void testUpdateIndexAttributesNoFlagSpecified() { .call(); } + @Test + public void testUpdateIndexOverwriteSameAttribute() { + createIndex("x"); + + exception.expect(IllegalStateException.class); + exception.expectMessage("Nothing to update..."); + geogig.command(UpdateIndexOp.class)// + .setTreeRefSpec(worldPointsLayer.getName())// + .setExtraAttributes(Lists.newArrayList("x"))// + .setOverwrite(true)// + .call(); + } + + @Test + public void testUpdateIndexAddSameAttribute() { + createIndex("x", "y"); + + exception.expect(IllegalStateException.class); + exception.expectMessage("Nothing to update..."); + geogig.command(UpdateIndexOp.class)// + .setTreeRefSpec(worldPointsLayer.getName())// + .setExtraAttributes(Lists.newArrayList("x"))// + .setAdd(true)// + .call(); + } + + @Test + public void testUpdateIndexDoNothing() { + createIndex("x"); + + exception.expect(IllegalStateException.class); + exception.expectMessage("Nothing to update..."); + geogig.command(UpdateIndexOp.class)// + .setTreeRefSpec(worldPointsLayer.getName())// + .call(); + } + @Test public void testUpdateNoExistingIndex() { exception.expect(IllegalStateException.class); @@ -442,9 +487,10 @@ private Index updateIndex(String treeName, @Nullable String... extraAttributes) extraAtts = Lists.newArrayList(extraAttributes); } Index index = geogig.command(UpdateIndexOp.class)// - .setAdd(true)// + .setOverwrite(true)// .setTreeRefSpec(treeName)// .setExtraAttributes(extraAtts)// + .setBounds(new Envelope(-180, 180, -90, 90))// .call(); return index; } diff --git a/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/CreateIndex.java b/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/CreateIndex.java index 071611464a..23b5605307 100644 --- a/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/CreateIndex.java +++ b/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/CreateIndex.java @@ -15,6 +15,7 @@ import org.locationtech.geogig.porcelain.index.CreateQuadTree; import org.locationtech.geogig.porcelain.index.Index; import org.locationtech.geogig.repository.Repository; +import org.locationtech.geogig.repository.impl.SpatialOps; import org.locationtech.geogig.web.api.AbstractWebAPICommand; import org.locationtech.geogig.web.api.CommandContext; import org.locationtech.geogig.web.api.CommandResponse; @@ -24,6 +25,8 @@ import org.restlet.data.Method; import org.restlet.data.Status; +import com.vividsolutions.jts.geom.Envelope; + /** * The interface for the Create Quad Tree operation in GeoGig. * @@ -40,6 +43,8 @@ public class CreateIndex extends AbstractWebAPICommand { boolean indexHistory; + String bbox; + public CreateIndex(ParameterSet options) { super(options); setTreeRefSpec(options.getFirstValue("treeRefSpec", null)); @@ -51,6 +56,7 @@ public CreateIndex(ParameterSet options) { setExtraAttributes(Arrays.asList(extraAttributes)); } setIndexHistory(Boolean.valueOf(options.getFirstValue("indexHistory", "false"))); + setBBox(options.getFirstValue("bounds", null)); } public void setTreeRefSpec(String treeRefSpec) { @@ -69,6 +75,10 @@ public void setIndexHistory(boolean indexHistory) { this.indexHistory = indexHistory; } + public void setBBox(String bbox) { + this.bbox = bbox; + } + @Override public boolean supports(final Method method) { return Method.PUT.equals(method); @@ -94,11 +104,15 @@ public boolean requiresTransaction() { @Override protected void runInternal(CommandContext context) { Repository repository = context.getRepository(); + + Envelope bounds = SpatialOps.parseNonReferencedBBOX(bbox); + final Index index = repository.command(CreateQuadTree.class)// .setTreeRefSpec(treeRefSpec)// .setGeometryAttributeName(geometryAttributeName)// .setExtraAttributes(extraAttributes)// .setIndexHistory(indexHistory)// + .setBounds(bounds)// .call(); context.setResponseContent(new CommandResponse() { diff --git a/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/UpdateIndex.java b/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/UpdateIndex.java index 478166aec1..7178cdedd2 100644 --- a/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/UpdateIndex.java +++ b/src/web/api/src/main/java/org/locationtech/geogig/web/api/index/UpdateIndex.java @@ -15,6 +15,7 @@ import org.locationtech.geogig.porcelain.index.Index; import org.locationtech.geogig.porcelain.index.UpdateIndexOp; import org.locationtech.geogig.repository.Repository; +import org.locationtech.geogig.repository.impl.SpatialOps; import org.locationtech.geogig.web.api.AbstractWebAPICommand; import org.locationtech.geogig.web.api.CommandContext; import org.locationtech.geogig.web.api.CommandResponse; @@ -24,6 +25,8 @@ import org.restlet.data.Method; import org.restlet.data.Status; +import com.vividsolutions.jts.geom.Envelope; + /** * The interface for the Update Index operation in GeoGig. * @@ -44,6 +47,8 @@ public class UpdateIndex extends AbstractWebAPICommand { boolean add; + String bbox; + public UpdateIndex(ParameterSet options) { super(options); setTreeRefSpec(options.getFirstValue("treeRefSpec", null)); @@ -57,6 +62,7 @@ public UpdateIndex(ParameterSet options) { setIndexHistory(Boolean.valueOf(options.getFirstValue("indexHistory", "false"))); setAdd(Boolean.valueOf(options.getFirstValue("add", "false"))); setOverwrite(Boolean.valueOf(options.getFirstValue("overwrite", "false"))); + setBBox(options.getFirstValue("bounds", null)); } public void setTreeRefSpec(String treeRefSpec) { @@ -83,6 +89,10 @@ public void setOverwrite(boolean overwrite) { this.overwrite = overwrite; } + public void setBBox(String bbox) { + this.bbox = bbox; + } + @Override public boolean supports(final Method method) { return Method.POST.equals(method); @@ -108,6 +118,9 @@ public boolean requiresTransaction() { @Override protected void runInternal(CommandContext context) { Repository repository = context.getRepository(); + + Envelope bounds = SpatialOps.parseNonReferencedBBOX(bbox); + final Index index = repository.command(UpdateIndexOp.class)// .setTreeRefSpec(treeRefSpec)// .setAttributeName(geometryAttributeName)// @@ -115,6 +128,7 @@ protected void runInternal(CommandContext context) { .setIndexHistory(indexHistory)// .setAdd(add)// .setOverwrite(overwrite)// + .setBounds(bounds)// .call(); context.setResponseContent(new CommandResponse() { diff --git a/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/CreateIndexTest.java b/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/CreateIndexTest.java index cb17c1f6a3..4bfec56620 100644 --- a/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/CreateIndexTest.java +++ b/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/CreateIndexTest.java @@ -10,7 +10,6 @@ package org.locationtech.geogig.web.api.index; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import javax.json.JsonObject; @@ -29,6 +28,7 @@ import org.locationtech.geogig.web.api.WebAPICommand; import com.google.common.base.Optional; +import com.vividsolutions.jts.geom.Envelope; public class CreateIndexTest extends AbstractWebOpTest { @@ -57,7 +57,7 @@ protected boolean requiresTransaction() { public void testBuildParameters() { ParameterSet options = TestParams.of("treeRefSpec", "points", "geometryAttributeName", "the_geom", "indexHistory", "true", "extraAttributes", "sp", "extraAttributes", - "ip"); + "ip", "bounds", "-90,-90,90,90"); CreateIndex op = (CreateIndex) buildCommand(options); assertEquals("points", op.treeRefSpec); @@ -66,6 +66,7 @@ public void testBuildParameters() { assertTrue(op.extraAttributes.contains("sp")); assertTrue(op.extraAttributes.contains("ip")); assertEquals(2, op.extraAttributes.size()); + assertEquals("-90,-90,90,90", op.bbox); } @Test @@ -82,13 +83,58 @@ public void testCreateIndex() throws Exception { buildCommand(options).run(testContext.get()); + Envelope expectedBounds = new Envelope(-90, 90, -180, 180); + + JsonObject response = getJSONResponse().getJsonObject("response"); + assertTrue(response.getBoolean("success")); + JsonObject index = response.getJsonObject("index"); + assertEquals("Points", index.getString("treeName")); + assertEquals("geom", index.getString("attributeName")); + assertEquals("QUADTREE", index.getString("indexType")); + assertEquals(expectedBounds.toString(), index.getString("bounds")); + ObjectId treeId = ObjectId.valueOf(response.getString("indexedTreeId")); + + IndexInfo indexInfo = geogig.indexDatabase().getIndexInfo("Points", "geom").get(); + assertEquals("Points", indexInfo.getTreeName()); + assertEquals("geom", indexInfo.getAttributeName()); + assertEquals(IndexType.QUADTREE, indexInfo.getIndexType()); + assertTrue(indexInfo.getMetadata().containsKey(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA)); + String[] extraAttributes = (String[]) indexInfo.getMetadata() + .get(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA); + assertEquals(1, extraAttributes.length); + assertEquals("sp", extraAttributes[0]); + + Optional indexedTreeId = geogig.indexDatabase().resolveIndexedTree(indexInfo, + canonicalFeatureTreeId); + assertTrue(indexedTreeId.isPresent()); + + assertEquals(indexedTreeId.get(), treeId); + } + + @Test + public void testCreateIndexWithBounds() throws Exception { + Repository geogig = testContext.get().getRepository(); + TestData testData = new TestData(geogig); + testData.init(); + testData.loadDefaultData(); + + ObjectId canonicalFeatureTreeId = geogig.command(ResolveTreeish.class) + .setTreeish("HEAD:Points").call().get(); + + ParameterSet options = TestParams.of("treeRefSpec", "Points", "extraAttributes", "sp", + "bounds", "-60,-45,60,45"); + + buildCommand(options).run(testContext.get()); + + Envelope expectedBounds = new Envelope(-60, 60, -45, 45); + JsonObject response = getJSONResponse().getJsonObject("response"); assertTrue(response.getBoolean("success")); JsonObject index = response.getJsonObject("index"); assertEquals("Points", index.getString("treeName")); assertEquals("geom", index.getString("attributeName")); assertEquals("QUADTREE", index.getString("indexType")); - assertNotNull(index.getString("bounds")); + assertEquals(expectedBounds.toString(), index.getString("bounds")); ObjectId treeId = ObjectId.valueOf(response.getString("indexedTreeId")); IndexInfo indexInfo = geogig.indexDatabase().getIndexInfo("Points", "geom").get(); @@ -123,13 +169,15 @@ public void testCreateIndexFullHistory() throws Exception { buildCommand(options).run(testContext.get()); + Envelope expectedBounds = new Envelope(-90, 90, -180, 180); + JsonObject response = getJSONResponse().getJsonObject("response"); assertTrue(response.getBoolean("success")); JsonObject index = response.getJsonObject("index"); assertEquals("Points", index.getString("treeName")); assertEquals("geom", index.getString("attributeName")); assertEquals("QUADTREE", index.getString("indexType")); - assertNotNull(index.getString("bounds")); + assertEquals(expectedBounds.toString(), index.getString("bounds")); ObjectId treeId = ObjectId.valueOf(response.getString("indexedTreeId")); IndexInfo indexInfo = geogig.indexDatabase().getIndexInfo("Points", "geom").get(); diff --git a/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/UpdateIndexTest.java b/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/UpdateIndexTest.java index 1524fcde71..e2b6410b94 100644 --- a/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/UpdateIndexTest.java +++ b/src/web/api/src/test/java/org/locationtech/geogig/web/api/index/UpdateIndexTest.java @@ -10,6 +10,7 @@ package org.locationtech.geogig.web.api.index; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -34,6 +35,7 @@ import com.google.common.base.Optional; import com.google.common.collect.Lists; +import com.vividsolutions.jts.geom.Envelope; public class UpdateIndexTest extends AbstractWebOpTest { @@ -185,6 +187,66 @@ public void testUpdateIndexFullHistory() throws Exception { assertTrue(indexedTreeId.isPresent()); } + @Test + public void testUpdateIndexBounds() throws Exception { + Repository geogig = testContext.get().getRepository(); + TestData testData = new TestData(geogig); + testData.init(); + testData.loadDefaultData(); + + geogig.command(CreateQuadTree.class).setTreeRefSpec("Points").call(); + + ObjectId canonicalFeatureTreeId = geogig.command(ResolveTreeish.class) + .setTreeish("HEAD:Points").call().get(); + + ParameterSet options = TestParams.of("treeRefSpec", "Points", "indexHistory", "true", + "bounds", "-60,-45,60,45"); + + buildCommand(options).run(testContext.get()); + + Envelope expectedBounds = new Envelope(-60, 60, -45, 45); + + JsonObject response = getJSONResponse().getJsonObject("response"); + assertTrue(response.getBoolean("success")); + JsonObject index = response.getJsonObject("index"); + assertEquals("Points", index.getString("treeName")); + assertEquals("geom", index.getString("attributeName")); + assertEquals("QUADTREE", index.getString("indexType")); + assertEquals(expectedBounds.toString(), index.getString("bounds")); + ObjectId treeId = ObjectId.valueOf(response.getString("indexedTreeId")); + + IndexInfo indexInfo = geogig.indexDatabase().getIndexInfo("Points", "geom").get(); + assertEquals("Points", indexInfo.getTreeName()); + assertEquals("geom", indexInfo.getAttributeName()); + assertEquals(IndexType.QUADTREE, indexInfo.getIndexType()); + assertFalse(indexInfo.getMetadata().containsKey(IndexInfo.FEATURE_ATTRIBUTES_EXTRA_DATA)); + + Optional indexedTreeId = geogig.indexDatabase().resolveIndexedTree(indexInfo, + canonicalFeatureTreeId); + assertTrue(indexedTreeId.isPresent()); + + assertEquals(indexedTreeId.get(), treeId); + + // make sure old commits are indexed + canonicalFeatureTreeId = geogig.command(ResolveTreeish.class).setTreeish("HEAD~1:Points") + .call().get(); + indexedTreeId = geogig.indexDatabase().resolveIndexedTree(indexInfo, + canonicalFeatureTreeId); + assertTrue(indexedTreeId.isPresent()); + + canonicalFeatureTreeId = geogig.command(ResolveTreeish.class).setTreeish("branch1:Points") + .call().get(); + indexedTreeId = geogig.indexDatabase().resolveIndexedTree(indexInfo, + canonicalFeatureTreeId); + assertTrue(indexedTreeId.isPresent()); + + canonicalFeatureTreeId = geogig.command(ResolveTreeish.class).setTreeish("branch2:Points") + .call().get(); + indexedTreeId = geogig.indexDatabase().resolveIndexedTree(indexInfo, + canonicalFeatureTreeId); + assertTrue(indexedTreeId.isPresent()); + } + @Test public void testUpdateIndexExistingAttributes() throws Exception { Repository geogig = testContext.get().getRepository(); @@ -235,6 +297,23 @@ public void testUpdateIndexWrongAttribute() throws Exception { buildCommand(options).run(testContext.get()); } + @Test + public void testUpdateIndexNothingToChange() throws Exception { + Repository geogig = testContext.get().getRepository(); + TestData testData = new TestData(geogig); + testData.init(); + testData.loadDefaultData(); + + geogig.command(CreateQuadTree.class).setTreeRefSpec("Points") + .setExtraAttributes(Lists.newArrayList("sp")).call(); + + ParameterSet options = TestParams.of("treeRefSpec", "Points"); + + ex.expect(IllegalStateException.class); + ex.expectMessage("Nothing to update..."); + buildCommand(options).run(testContext.get()); + } + @Test public void testUpdateIndexExistingAttributesAdd() throws Exception { Repository geogig = testContext.get().getRepository(); diff --git a/src/web/functional/src/test/java/org/geogig/web/functional/WebAPICucumberHooks.java b/src/web/functional/src/test/java/org/geogig/web/functional/WebAPICucumberHooks.java index 1bcaa75704..06b48b219e 100644 --- a/src/web/functional/src/test/java/org/geogig/web/functional/WebAPICucumberHooks.java +++ b/src/web/functional/src/test/java/org/geogig/web/functional/WebAPICucumberHooks.java @@ -80,6 +80,7 @@ import org.locationtech.geogig.repository.NodeRef; import org.locationtech.geogig.repository.Repository; import org.locationtech.geogig.repository.impl.GeogigTransaction; +import org.locationtech.geogig.repository.impl.SpatialOps; import org.locationtech.geogig.rest.AsyncContext; import org.locationtech.geogig.rest.Variants; import org.locationtech.geogig.rest.geopkg.GeoPackageWebAPITestSupport; @@ -109,6 +110,7 @@ import com.google.common.collect.Sets; import com.google.common.io.ByteStreams; import com.google.inject.Inject; +import com.vividsolutions.jts.geom.Envelope; import cucumber.api.DataTable; import cucumber.api.java.en.Given; @@ -409,6 +411,24 @@ public void verifyIndexNotExtraAttributes(String repositoryName, String headRef, openedRepos.add(repositoryName); } + @Then("^the ([^\"]*) repository's \"([^\"]*)\" index bounds should be \"([^\"]*)\"$") + public void verifyIndexBounds(String repositoryName, String headRef, String bbox) + throws Throwable { + Repository repo = context.getRepo(repositoryName); + final NodeRef typeTreeRef = IndexUtils.resolveTypeTreeRef(repo.context(), headRef); + String treeName = typeTreeRef.path(); + IndexInfo indexInfo = IndexUtils.resolveIndexInfo(repo.indexDatabase(), treeName, null); + Map metadata = indexInfo.getMetadata(); + assertTrue(metadata.containsKey(IndexInfo.MD_QUAD_MAX_BOUNDS)); + Envelope indexBounds = (Envelope) metadata.get(IndexInfo.MD_QUAD_MAX_BOUNDS); + Envelope expected = SpatialOps.parseNonReferencedBBOX(bbox); + final double EPSILON = 0.00001; + assertEquals(expected.getMinX(), indexBounds.getMinX(), EPSILON); + assertEquals(expected.getMaxX(), indexBounds.getMaxX(), EPSILON); + assertEquals(expected.getMinY(), indexBounds.getMinY(), EPSILON); + assertEquals(expected.getMaxY(), indexBounds.getMaxY(), EPSILON); + } + @Given("^There are multiple branches on the \"([^\"]*)\" repo$") public void There_are_multiple_branches(String repoName) throws Throwable { Repository repo = context.getRepo(repoName); diff --git a/src/web/functional/src/test/resources/features/commands/IndexCreate.feature b/src/web/functional/src/test/resources/features/commands/IndexCreate.feature index eedf473cc3..df4f0f949b 100644 --- a/src/web/functional/src/test/resources/features/commands/IndexCreate.feature +++ b/src/web/functional/src/test/resources/features/commands/IndexCreate.feature @@ -26,9 +26,10 @@ Feature: IndexCreate Scenario: Verify success after adding spatial index Given There is a repo with some data When I call "PUT /repos/repo1/index/create?treeRefSpec=Points" - And the xpath "/response/success/text()" equals "true" + Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" - Then the response status should be '201' + And the response status should be '201' + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should have the following features: | index | | Point.1 | @@ -39,11 +40,25 @@ Feature: IndexCreate Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" And the response status should be '201' + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: | index | | Point.1 | + + Scenario: Verify creating index with bounds + Given There is a repo with some data + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points&bounds=-60,-45,60,45" + Then the xpath "/response/success/text()" equals "true" + And the xpath "/response/index/treeName/text()" equals "Points" + And the response status should be '201' + And the repo1 repository's "HEAD:Points" index bounds should be "-60,-45,60,45" + And the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" + And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repo1 repository's "HEAD:Points" index should have the following features: + | index | + | Point.1 | Scenario: Verify creating index with extra attribute Given There is a repo with some data @@ -51,6 +66,7 @@ Feature: IndexCreate Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" And the response status should be '201' + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: @@ -63,6 +79,7 @@ Feature: IndexCreate Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" And the response status should be '201' + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should have the following features: | index | | Point.1 | @@ -86,6 +103,7 @@ Feature: IndexCreate Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" And the response status should be '201' + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: diff --git a/src/web/functional/src/test/resources/features/commands/IndexUpdate.feature b/src/web/functional/src/test/resources/features/commands/IndexUpdate.feature index 62dc6c0e9e..de4d6ce1cb 100644 --- a/src/web/functional/src/test/resources/features/commands/IndexUpdate.feature +++ b/src/web/functional/src/test/resources/features/commands/IndexUpdate.feature @@ -17,8 +17,9 @@ Feature: IndexUpdate Scenario: Verify updating spatial index by adding attributes Given There is a repo with some data - And I call "PUT /repos/repo1/index/create?treeRefSpec=Points" - Then the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points" + Then the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" + And the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: | index | @@ -44,8 +45,9 @@ Feature: IndexUpdate Scenario: Verify success when overwriting the attributes of an index Given There is a repo with some data - And I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp" - Then the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp" + Then the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" + And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: | index | @@ -53,6 +55,7 @@ Feature: IndexUpdate When I call "POST /repos/repo1/index/update?treeRefSpec=Points&extraAttributes=ip&overwrite=true" Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: @@ -60,10 +63,11 @@ Feature: IndexUpdate | Point.1 | And the response status should be '201' - Scenario: Verify success when removing the attributes of an index + Scenario: Verify success when removing the attributes of an index Given There is a repo with some data - And I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp" - Then the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp" + Then the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" + And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: | index | @@ -71,6 +75,7 @@ Feature: IndexUpdate When I call "POST /repos/repo1/index/update?treeRefSpec=Points&overwrite=true" Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: @@ -78,10 +83,31 @@ Feature: IndexUpdate | Point.1 | And the response status should be '201' + Scenario: Verify success when updating the bounds + Given There is a repo with some data + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp" + Then the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" + And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" + And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repo1 repository's "HEAD:Points" index should have the following features: + | index | + | Point.1 | + When I call "POST /repos/repo1/index/update?treeRefSpec=Points&bounds=-60,-45,60,45" + Then the xpath "/response/success/text()" equals "true" + And the xpath "/response/index/treeName/text()" equals "Points" + And the repo1 repository's "HEAD:Points" index bounds should be "-60,-45,60,45" + And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" + And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" + And the repo1 repository's "HEAD:Points" index should have the following features: + | index | + | Point.1 | + And the response status should be '201' + Scenario: Verify success when updating the whole history of an index Given There is a default multirepo server - And I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp&indexHistory=true" - Then the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp&indexHistory=true" + Then the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" + And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: | index | @@ -108,6 +134,7 @@ Feature: IndexUpdate When I call "POST /repos/repo1/index/update?treeRefSpec=Points&extraAttributes=ip&overwrite=true&indexHistory=true" Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: @@ -137,8 +164,9 @@ Feature: IndexUpdate Scenario: Verify success when updating only the head commit of an index Given There is a default multirepo server - And I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp&indexHistory=true" - Then the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" + When I call "PUT /repos/repo1/index/create?treeRefSpec=Points&extraAttributes=sp&indexHistory=true" + Then the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" + And the repo1 repository's "HEAD:Points" index should track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: | index | @@ -165,6 +193,7 @@ Feature: IndexUpdate When I call "POST /repos/repo1/index/update?treeRefSpec=Points&extraAttributes=ip&overwrite=true" Then the xpath "/response/success/text()" equals "true" And the xpath "/response/index/treeName/text()" equals "Points" + And the repo1 repository's "HEAD:Points" index bounds should be "-90,-180,90,180" And the repo1 repository's "HEAD:Points" index should not track the extra attribute "sp" And the repo1 repository's "HEAD:Points" index should track the extra attribute "ip" And the repo1 repository's "HEAD:Points" index should have the following features: