Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

#788 Add flags for poi search, update documentation. #1010

Merged
merged 4 commits into from Sep 27, 2018
Merged

#788 Add flags for poi search, update documentation. #1010

merged 4 commits into from Sep 27, 2018

Conversation

CTJeffries
Copy link
Contributor

@CTJeffries CTJeffries commented Sep 25, 2018

Added flags to geserveradmin to allow publishing databases with POI search information. Updated the documentation of geserveradmin to reflect these new flags.

Automatic testing these changes would require rewriting PublisherClient.cpp, and testing the command line utility itself would require an additional testing framework outside of gtest. Because of the lack of automatic testing, manual test instructions are included below.

To test POI search flag for geserveradmin:

Create a test database, my test database used the 4km bluemarble imagery data and the California population vector data. Add POI information making sure it is searchable, I added the names of the cities.

Test 1) Publish the database using:
geserveradmin --publishdb <db_name> --targetpath <publish_name>
Upon command success, open up the earth client. Try to search something, you should get no results as POI search is not enabled. Unpublish the database using either console commands or the geserver webpage.

Test 2) Publish the database using:
geserveradmin --publishdb <db_name> --targetpath <publish_name> --enable_poisearch
Upon command success, open up the earth client. Search for something that is in your POI data (In my case I search "san", as there are many cities in California with "san" in their name). You should get results that are only in your database. Unpublish the database using either console commands or the geserver webpage.

Test 3) Publish the database using:
geserveradmin --publishdb <db_name> --targetpath <publish_name> --enable_poisearch --enable_enhancedsearch
Open up the earth client. Search for something that is not in your POI data, perhaps a city in the US (In my case I search "Wooster", as there is no city in California called Wooster, but there are a few scattered throughout the rest of the US). You should get results for data outside your database using the GeocodingFederated plug-in. Unpublish the database using either console commands or the geserver webpage.

Test 4) Attempt to publish the database using:
geserveradmin --publishdb <db_name> --targetpath <publish_name> --enable_enhancedsearch
You should get a fusion fatal error, indicating that you cannot enable enhanced search without POI search being enabled.

Fixes #788

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@CTJeffries
Copy link
Contributor Author

I signed it.

@googlebot
Copy link

CLAs look good, thanks!

@tst-lsavoie
Copy link
Collaborator

@CTJeffries the docs that need to be updated are located at https://github.com/google/earthenterprise/blob/master/docs/geedocs/5.2.5/answer/3481558.html. There's a section on geserveradmin that needs to have the new parameters added.

You may need to rebase your branch on the latest master to get access to the 5.2.5 version of the docs. Ping me if you'd like help with that.

@CTJeffries
Copy link
Contributor Author

Changes to the documentation have been added, along with the addition requested check.

Copy link
Collaborator

@tst-rwildes tst-rwildes left a comment

Choose a reason for hiding this comment

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

Nice clean code. I'm approving the PR. Did not test.

Copy link
Collaborator

@tst-lsavoie tst-lsavoie left a comment

Choose a reason for hiding this comment

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

Reviewed and tested - everything looks good. Good work!

@tst-lsavoie tst-lsavoie merged commit f681f82 into google:master Sep 27, 2018
Copy link
Collaborator

@tst-nfarah-zz tst-nfarah-zz left a comment

Choose a reason for hiding this comment

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

husf-dsheremata added a commit to husf-dsheremata/earthenterprise that referenced this pull request Jan 11, 2019
* 'master' of https://github.com/google/earthenterprise:
  google#997: Allowing alpha version tag on master branch to enable parallel development of current verison on release branch, and next release on master (google#1029)
  Fixes google#1004: don't read version file for tasks that don't need to read it (google#1005)
  google#788 Add flags for poi search, update documentation. (google#1010)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants