protobuf: refactor proto visitor pattern. #10354
Merged
Conversation
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
Move the proto traversal handling in version_converter.cc to a standalone library. This lets us replace existing proto visitor patterns in common/protobuf/utility.cc for unexpected field checks. The redaction code is actually a bit more involved, so I'm not refactoring this; it needs to recurse through Any/TypedStruct. Ultimately we might want something like this, but it doesn't seem super helpful given we only have a the single instance of this right now. Risk level: Low Testing: Existing tests continue to pass. Signed-off-by: Harvey Tuch <[email protected]>
|
@alyssawilk this continues #9807; I didn't seem to be able to reopen that PR. |
alyssawilk
approved these changes
Mar 13, 2020
| } | ||
| // We use the validation visitor but have hard coded behavior below for deprecated fields. | ||
| // TODO(htuch): Unify the deprecated and unknown visitor handling behind the validation | ||
| // visitor pattern. https://github.com/envoyproxy/envoy/issues/8092. |
There was a problem hiding this comment.
Checking: you still have further cleanup plans?
There was a problem hiding this comment.
Yeah. This PR just refactors some proto traversal stuff. #8092 tracks the more general "move the deprecated field error reporting and exception generation to the validation visitor callbacks", i.e. having an onDeprecatedField() callback alongside onUnknownField() in ValidationVisitor. This is mostly a mechanical code movement that shouldn't really change any existing behavior. @nezdolik FYI.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Move the proto traversal handling in version_converter.cc to a
standalone library. This lets us replace existing proto visitor patterns in
common/protobuf/utility.cc for unexpected field checks.
The redaction code is actually a bit more involved, so I'm not
refactoring this; it needs to recurse through Any/TypedStruct.
Ultimately we might want something like this, but it doesn't seem super
helpful given we only have a the single instance of this right now.
Risk level: Low
Testing: Existing tests continue to pass.
Signed-off-by: Harvey Tuch [email protected]