-
Notifications
You must be signed in to change notification settings - Fork 657
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
Add serialized warnings array to JSON output #3588
Merged
Merged
Changes from 81 commits
Commits
Show all changes
90 commits
Select commit
Hold shift + click to select a range
f6a577f
[src/worker.cc] add warnings for deprecated auto-shorter
MuluhGodson 28cec35
[tyr/serializers.cc] function to serialize warnings
MuluhGodson a0edec8
[serializers.h] declare function for serializeWarnings
MuluhGodson 6ad5852
[serializers.cc] refactor function for serializeWarnings
MuluhGodson 7a52fc5
[route_serializer_valhalla.cc] add warnings serializing function to r…
MuluhGodson d85b06e
[serializer.cc] add warning type
MuluhGodson 2a5154e
[warning] add warning for deprecated auto shorter
MuluhGodson c39ea82
[warning] add warning for deprecated hov costing
MuluhGodson c53b702
[warning] add waring for deprecated auto_data_fix
MuluhGodson 9c3f845
[warning] add warning for deprecated best_paths
MuluhGodson a1387e0
[tyr/serializers.cc] refactor serializeWarnings function
MuluhGodson ec1370a
[src/worker.cc] refactor warnings object into a function
MuluhGodson eba2c5c
[tyr/serializer] convert warnings array object to flat array
MuluhGodson d44404e
make warning_text const
MuluhGodson 869cfd2
overload serializeWarnings function for baldr/json.h
MuluhGodson b20f996
make warning_text a reference
MuluhGodson aec22b6
add function overload for serializeWarnings for other endpoints
MuluhGodson 042c5ba
[isochrone_serializer.cc] add warnings to json output
MuluhGodson 94f92b4
[locate_serializer.cc] add warning to locate endpoint json output
MuluhGodson b4d3361
[matrix_serializer.cc] add warnings to json response
MuluhGodson cf9da92
[height_serializer.cc] add warnings to json response
MuluhGodson acdb943
[transit_available_serializer.cc] add warnings to json output
MuluhGodson 8d0878f
[src/tyr/serializers.cc] refactor serializeWarnings function to remov…
MuluhGodson e1f5208
[trace_serializer.cc] add warnings to json output
MuluhGodson 5d997e0
refactor function to serialize warnings
MuluhGodson fecb297
[src/worker.cc] make best_paths warning only valid for request with b…
MuluhGodson 453cdd6
prevent json response from containing an empty warnings array
MuluhGodson 231bba5
[test] add test for route endpoint warnings
MuluhGodson 817b58a
add array of deprecated costing options for test cases
MuluhGodson a65c191
add test case for locate endpoint
MuluhGodson e2b17f0
loop through deprecated costing options for route test case
MuluhGodson 5e2560c
add test case for isochrone endpoint
MuluhGodson e6c4f43
add test case for transit_available endpoint
MuluhGodson 52049c8
add test case for heights endpoint
MuluhGodson acfaeb1
add test case for map_matching endpoints
MuluhGodson 51d1290
add test case for matrix endpoint
MuluhGodson 24fb7f3
change add warnings function to lambda function
MuluhGodson 1a82c87
[routes] update documentation to include info about warnings
MuluhGodson 3e7a32f
[transit_available] update documentation with information about warnings
MuluhGodson 7295285
[matrix] update documentation with info about warnings
MuluhGodson 507274b
[map-matching] update documentation add information about warnings
MuluhGodson 834dc06
[locate] update documentation with information about warnings
MuluhGodson efb7ee5
[isochrone] update documentation with information about warnings
MuluhGodson 09d6ce1
[elevation] update documentation with information about warnings
MuluhGodson 5e9f246
code clean up - remove superflfous proto includes
MuluhGodson 6cedf81
move lambda add_warnings function to worker.h
MuluhGodson f0fdaf2
update CHANGELOG.md with added enhancements
MuluhGodson bd77452
remove loop indexes from route warnings array
MuluhGodson b2b1ada
convert warnings function to a normal function
MuluhGodson 4ba4d4f
Merge branch 'master' into mg_warnings_array
MuluhGodson 63779ce
add unordered map for warning pairs
MuluhGodson 2dfc963
modify add_warnings function to utilizie unordered_map for warning pairs
MuluhGodson 7b725da
modify warnings messages to use unorderd_map for warning pairs
MuluhGodson 9119c81
add codes to serialized warnings
MuluhGodson a188925
Merge branch 'mg_warnings_array' of https://github.com/MuluhGodson/va…
MuluhGodson c660995
Merge branch 'master' into mg_warnings_array
MuluhGodson 1a81f8a
add more context to the warnings array output.
MuluhGodson acfdfda
add more context to warnings array output.
MuluhGodson 2fc8ac3
update to add more context to warnings array output
MuluhGodson cb8062c
add more context to warnings array output
MuluhGodson ec19dd8
add more context to warnings array
MuluhGodson 91d1937
add more context to warnings json
MuluhGodson e2634c6
move function definition and warning codes to worker.cc
MuluhGodson f7a9f4a
rewrite tests to all use one map
MuluhGodson f9e2834
change variable name from i to costing for looping costing methods
MuluhGodson 67dec12
refactor test_warnings.cc to parametized gtest
MuluhGodson c7ab2ae
revert test cases
MuluhGodson 3e0a19b
remove warnings from transit endpoint
MuluhGodson bec286e
remove warnings test for transit endpoint
MuluhGodson 40e10ca
Merge remote-tracking branch 'upstream/master' into mg_warnings_array
nilsnolde 76586e6
finish off PR
nilsnolde 1509b97
lint & format; change some docs stuff
nilsnolde 027d22c
untidy
nilsnolde 6cd7d63
Merge remote-tracking branch 'upstream/master' into mg_warnings_array
nilsnolde 524639b
Merge remote-tracking branch 'upstream/master' into mg_warnings_array
nilsnolde d63423f
changelog update
nilsnolde 9b9ad42
Update src/tyr/serializers.cc
nilsnolde 4c5a868
Apply suggestions from code review
nilsnolde 233f923
github UI conflict resolve failed format.sh
nilsnolde 1836a4f
Merge remote-tracking branch 'upstream/master' into mg_warnings_array
nilsnolde 70da775
raw strings instead of escaping quotes
nilsnolde e6d6a35
changelog
nilsnolde 07b1f41
typo
nilsnolde 4aa63e2
Merge branch 'mg_warnings_array' of https://github.com/MuluhGodson/va…
nilsnolde 75c3aea
revert gurka changes
nilsnolde 1b3cc09
Merge remote-tracking branch 'upstream/master' into mg_warnings_array
nilsnolde a82eddb
changelog
nilsnolde 145f001
wtf
nilsnolde e624e2f
still..
nilsnolde 9b84b39
remove redundant include
nilsnolde File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,9 +64,9 @@ std::vector<midgard::PointLL> to_lls(const nodelayout& nodes, | |
*/ | ||
std::string build_valhalla_request(const std::string& location_type, | ||
const std::vector<midgard::PointLL>& waypoints, | ||
const std::string& costing = "auto", | ||
const std::unordered_map<std::string, std::string>& options = {}, | ||
const std::string& stop_type = "break") { | ||
const std::string& costing, | ||
const std::unordered_map<std::string, std::string>& options, | ||
const std::string& stop_type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made this function public so it can be used in tests. Hope that's fine with you.. |
||
|
||
rapidjson::Document doc; | ||
doc.SetObject(); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
#include "gurka.h" | ||
#include <gtest/gtest.h> | ||
#include <vector> | ||
|
||
using namespace valhalla; | ||
|
||
TEST(Warning, DeprecatedParams) { | ||
for (auto& costing : {"auto_shorter", "hov", "auto_data_fix"}) { | ||
Api request; | ||
std::string request_str = | ||
valhalla::gurka::detail::build_valhalla_request("locations", {{1.0, 1.0}, {2.0, 2.0}}, | ||
costing, {{"/best_paths", "2"}}); | ||
ParseApi(request_str, Options::route, request); | ||
EXPECT_EQ(request.info().warnings_size(), 2); | ||
} | ||
} |
Submodule rapidjson
updated
from 73063f to b16cec
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"warnings"
is only returned if there was at least one warning, so it's conditionally present in JSON output. I'd personally prefer to output an empty array, I think it's nicer for end users in most scenarios. Other opinions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah thanks for bringing this up. I've done some more work with APIs and this is definitely a good way to go as some users may write their logic keeping in mind there's a warmings output and if not properly configures for null safety could be problematic.
I'll update the PR. I certainly have a lot to catch up on. 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries, I just carried it over the finish line, wasn't much to left to do:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks.