Add ConfigV2 Validator #2672

andreamlin · 2019-03-29T21:42:41Z

Add util to validate a ConfigProto object as valid under the V2 schema.

This util is currently not used, but it is planned to be used very soon.

codecov · 2019-03-29T21:44:26Z

Codecov Report

Merging #2672 into gapic_config_v2 will decrease coverage by <.01%.
The diff coverage is 82.75%.

@@                  Coverage Diff                  @@
##             gapic_config_v2    #2672      +/-   ##
=====================================================
- Coverage              86.77%   86.76%   -0.01%     
- Complexity              5584     5588       +4     
=====================================================
  Files                    466      467       +1     
  Lines                  22193    22222      +29     
  Branches                2428     2430       +2     
=====================================================
+ Hits                   19257    19281      +24     
- Misses                  2078     2081       +3     
- Partials                 858      860       +2

Impacted Files	Coverage Δ	Complexity Δ
...e/api/codegen/util/ConfigNextVersionValidator.java	`82.75% <82.75%> (ø)`	`4 <4> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 280e279...98fe47a. Read the comment docs.

codecov · 2019-03-29T21:44:26Z

Codecov Report

Merging #2672 into gapic_config_v2 will decrease coverage by <.01%.
The diff coverage is 84.21%.

@@                  Coverage Diff                  @@
##             gapic_config_v2    #2672      +/-   ##
=====================================================
- Coverage              86.76%   86.76%   -0.01%     
- Complexity              5584     5589       +5     
=====================================================
  Files                    466      467       +1     
  Lines                  22185    22204      +19     
  Branches                2428     2430       +2     
=====================================================
+ Hits                   19249    19265      +16     
- Misses                  2078     2080       +2     
- Partials                 858      859       +1

Impacted Files	Coverage Δ	Complexity Δ
...oogle/api/codegen/util/ConfigVersionValidator.java	`84.21% <84.21%> (ø)`	`5 <5> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1214ae5...43a9a6c. Read the comment docs.

andreamlin · 2019-03-29T21:45:10Z

PTAL

michaelbausor · 2019-03-29T22:09:35Z

src/main/java/com/google/api/codegen/util/ConfigNextVersionValidator.java

+import com.google.protobuf.util.JsonFormat.TypeRegistry;
+import javax.annotation.Nonnull;
+
+public class ConfigNextVersionValidator {


This name is a bit odd. What about just ConfigVersionValidator?

michaelbausor · 2019-03-29T22:30:34Z

src/main/java/com/google/api/codegen/util/ConfigNextVersionValidator.java

+
+    try {
+      // Serializing utils for Config V2.
+      TypeRegistry typeRegistryV2 =


Is it necessary to use a TypeRegistry and json formatting functions? Can we not just use toByteArray, parseFrom(byte[]), and check for unknown fields?

If we use the toByteArray and parseFrom functions, those will still preserve the unknown fields. The thing with the unknown fields is that they can exist at every single level of the protobuf object tree, and I don't feel like writing a tree search function to parse through every child message type and looking for unknown types (i've already been burned trying to implement a BFS haha)

What about using a DiscardUnknownFieldsParser? I don't know, but my intuition is that there is an almost-one-liner that does this, something like:

configBytes = config.toByteArray(); configV2Bytes = DiscardUnknownFieldsParser.wrap(ConfigV2.parser()) .parseFrom(configBytes) .toByteArray(); // compare byte arrays ...

But - maybe I'm just wrong? Like, does the above fail because of unknown fields other than v1/v2 differences?

oh i didn't see that! i heard that Go had the same thing, but i didn't look hard enough for a java version

much shorter now :) i must have been looking at stackoverflow posts that are outdated now for help on this

michaelbausor · 2019-03-29T22:36:27Z

src/main/java/com/google/api/codegen/util/ConfigNextVersionValidator.java

+   * Throw {@link IllegalStateException} iff the given input contains fields unknown to the {@link
+   * com.google.api.codegen.v2.ConfigProto} schema.
+   */
+  public void checkIsNextVersionConfig(@Nonnull com.google.api.codegen.ConfigProto configV1Proto)


What about something like validateV2Config? I don't think it is desirable to make the function name version-agnostic (unless the implementation is also version-agnostic)

andreamlin · 2019-03-29T22:52:41Z

~~asking protobuf team to make a java release too protocolbuffers/protobuf#5972~~ wrong pr

andreamlin · 2019-03-29T22:57:48Z

renamed things
PTAL

andreamlin · 2019-04-01T04:15:22Z

PTAL

michaelbausor · 2019-04-01T15:15:36Z

src/main/java/com/google/api/codegen/util/ConfigVersionValidator.java

+    }
+
+    try {
+


Nit: remove extra line

michaelbausor · 2019-04-01T15:17:44Z

src/main/java/com/google/api/codegen/util/ConfigVersionValidator.java

+      if (!Arrays.equals(configV2.toByteArray(), configV1Proto.toByteArray())) {
+        throw new IllegalStateException(
+            String.format(
+                "Unknown fields in to ConfigProto v2 in configProto: %s",


s/in to ConfigProto/to ConfigProto/

* Add Gapic config v2 (#2665) * Whittling down config_v2 (#2666) * Add ConfigV2 Validator (#2672) * AutoValue LongRunningConfig; always use gapic config's polling settings (#2698) * ResourceNameOneofConfig fixes (#2704) * Start parsing GAPIC config v2 (#2703) * Bring back timeout millis in GAPIC config v2 (#2708) * Resource names across different protofiles (#2711) * Fix missing default retries (#2718) * Bug fixes for gapic config v2 parsing (#2717)

* Add Gapic config v2 (googleapis#2665) * Whittling down config_v2 (googleapis#2666) * Add ConfigV2 Validator (googleapis#2672) * AutoValue LongRunningConfig; always use gapic config's polling settings (googleapis#2698) * ResourceNameOneofConfig fixes (googleapis#2704) * Start parsing GAPIC config v2 (googleapis#2703) * Bring back timeout millis in GAPIC config v2 (googleapis#2708) * Resource names across different protofiles (googleapis#2711) * Fix missing default retries (googleapis#2718) * Bug fixes for gapic config v2 parsing (googleapis#2717)

add configv2 validator

8212282

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 29, 2019

javadoc

98fe47a

andreamlin requested a review from michaelbausor March 29, 2019 21:45

michaelbausor reviewed Mar 29, 2019

View reviewed changes

ConfigVersionValidator

fe7464a

michaelbausor reviewed Mar 29, 2019

View reviewed changes

validateV2Config

ab64503

andreamlin added 2 commits March 29, 2019 16:50

Merge branch 'gapic_config_v2' into validate_config_v2

9e5281a

use DiscardUnkonwnFieldsParser

56b0d28

michaelbausor approved these changes Apr 1, 2019

View reviewed changes

andreamlin added 2 commits April 1, 2019 08:47

cleanup

617fc9a

cleanup

43a9a6c

andreamlin merged commit 5ecc799 into googleapis:gapic_config_v2 Apr 1, 2019

andreamlin deleted the validate_config_v2 branch April 1, 2019 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ConfigV2 Validator #2672

Add ConfigV2 Validator #2672

andreamlin commented Mar 29, 2019 •

edited

Loading

codecov bot commented Mar 29, 2019

codecov bot commented Mar 29, 2019 •

edited

Loading

andreamlin commented Mar 29, 2019

michaelbausor Mar 29, 2019

andreamlin Mar 29, 2019

michaelbausor Mar 29, 2019

andreamlin Mar 29, 2019

michaelbausor Mar 29, 2019

michaelbausor Mar 29, 2019

andreamlin Apr 1, 2019

andreamlin Apr 1, 2019

michaelbausor Mar 29, 2019

andreamlin Mar 29, 2019

andreamlin commented Mar 29, 2019 •

edited

Loading

andreamlin commented Mar 29, 2019

andreamlin commented Apr 1, 2019

michaelbausor Apr 1, 2019

andreamlin Apr 1, 2019

michaelbausor Apr 1, 2019

andreamlin Apr 1, 2019

Add ConfigV2 Validator #2672

Add ConfigV2 Validator #2672

Conversation

andreamlin commented Mar 29, 2019 • edited Loading

codecov bot commented Mar 29, 2019

Codecov Report

codecov bot commented Mar 29, 2019 • edited Loading

Codecov Report

andreamlin commented Mar 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreamlin commented Mar 29, 2019 • edited Loading

andreamlin commented Mar 29, 2019

andreamlin commented Apr 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreamlin commented Mar 29, 2019 •

edited

Loading

codecov bot commented Mar 29, 2019 •

edited

Loading

andreamlin commented Mar 29, 2019 •

edited

Loading