feat: numeric enum option generation (#497)

* feat: numeric enum option generation * fix bazel rule * add missing numericEnums param * fix baselines * fix example target * exclude exmaple build file changes * address feedback Co-authored-by: Brent Shaffer <[email protected]>
googleapis · Jul 21, 2022 · b123f42 · b123f42
1 parent eb9cb61
commit b123f42
Show file tree

Hide file tree

Showing 19 changed files with 50 additions and 11 deletions.
diff --git a/rules_php_gapic/php_gapic.bzl b/rules_php_gapic/php_gapic.bzl
@@ -52,6 +52,7 @@ def php_gapic_srcjar(
         service_yaml = None,
         grpc_service_config = None,
         transport = None,
+        rest_numeric_enums = False,
         generator_binary = Label("//rules_php_gapic:php_gapic_generator_binary"),
         **kwargs):
     plugin_file_args = {}
@@ -70,9 +71,14 @@ def php_gapic_srcjar(
     if transport != "grpc+rest" and transport != "rest":
         fail("Error: Only 'grpc+rest' or 'rest' transports are supported")
 
+
     # Set plugin arguments.
     plugin_args = ["metadata"]  # Generate the gapic_metadata.json file.
     plugin_args.append("transport=%s" % transport)
+
+    # Generate REGAPIC param for requesting response enums be JSON-encoded as numbers, not strings.
+    if rest_numeric_enums:
+        plugin_args.append("rest-numeric-enums")
 
     proto_custom_library(
         name = name,
@@ -106,6 +112,7 @@ def php_gapic_library(
         service_yaml = None,
         grpc_service_config = None,
         transport = None,
+        rest_numeric_enums = False,
         generator_binary = Label("//rules_php_gapic:php_gapic_generator_binary"),
         **kwargs):
     srcjar_name = "%s_srcjar" % name
@@ -117,6 +124,7 @@ def php_gapic_library(
         service_yaml = service_yaml,
         grpc_service_config = grpc_service_config,
         transport = transport,
+        rest_numeric_enums = rest_numeric_enums,
         generator_binary = generator_binary,
         **kwargs
     )

diff --git a/src/CodeGenerator.php b/src/CodeGenerator.php
@@ -56,6 +56,7 @@ class CodeGenerator
      * @param ?string $grpcServiceConfigJson Optional grpc-serv-config json string.
      * @param ?string $gapicYaml Optional gapic configuration yaml string.
      * @param ?string $serviceYaml Optional service configuration yaml string.
+     * @param bool $numericEnums Whether to generate the numeric-enums JSON encoding system parameter.
      * @param int $licenseYear The year to use in license headers.
      *
      * @return string[]
@@ -68,6 +69,7 @@ public static function generateFromDescriptor(
         ?string $grpcServiceConfigJson,
         ?string $gapicYaml,
         ?string $serviceYaml,
+        bool $numericEnums = false,
         int $licenseYear = -1
     ) {
         $descSet = new FileDescriptorSet();
@@ -76,7 +78,7 @@ public static function generateFromDescriptor(
         $filesToGenerate = $fileDescs
             ->filter(fn ($x) => substr($x->getPackage(), 0, strlen($package)) === $package)
             ->map(fn ($x) => $x->getName());
-        yield from static::generate($fileDescs, $filesToGenerate, $transport, $generateGapicMetadata, $grpcServiceConfigJson, $gapicYaml, $serviceYaml, $licenseYear);
+        yield from static::generate($fileDescs, $filesToGenerate, $transport, $generateGapicMetadata, $grpcServiceConfigJson, $gapicYaml, $serviceYaml, $numericEnums, $licenseYear);
     }
 
     /**
@@ -91,6 +93,8 @@ public static function generateFromDescriptor(
      * @param ?string $grpcServiceConfigJson Optional grpc-serv-config json string.
      * @param ?string $gapicYaml Optional gapic configuration yaml string.
      * @param ?string $serviceYaml Optional service configuration yaml string.
+     * @param bool $numericEnums Optional whether to include in requests the system parameter enabling JSON-encoded
+     *     responses to encode enum values as numbers.
      * @param int $licenseYear The year to use in license headers.
      *
      * @return array[] [0] (string) is relative path; [1] (string) is file content.
@@ -103,6 +107,7 @@ public static function generate(
         ?string $grpcServiceConfigJson,
         ?string $gapicYaml,
         ?string $serviceYaml,
+        bool $numericEnums = false,
         int $licenseYear = -1
     ) {
         if ($licenseYear < 0) {
@@ -221,7 +226,8 @@ public static function generate(
             $gapicYaml,
             $serviceYamlConfig,
             $generateGapicMetadata,
-            $licenseYear
+            $licenseYear,
+            $numericEnums
         ) as $file) {
             $result[] = $file;
         }
@@ -245,7 +251,8 @@ private static function generateServices(
         ?string $gapicYaml,
         ?ServiceYamlConfig $serviceYamlConfig,
         bool $generateGapicMetadata,
-        int $licenseYear
+        int $licenseYear,
+        bool $numericEnums = false
     ) {
         $versionToNamespace = [];
         foreach ($servicesToGenerate as $service) {
@@ -306,7 +313,7 @@ private static function generateServices(
             $code = Formatter::format($code);
             yield ["src/{$version}resources/{$service->descriptorConfigFilename}", $code];
             // Resource: rest_client_config.php
-            $code = ResourcesGenerator::generateRestConfig($service, $serviceYamlConfig);
+            $code = ResourcesGenerator::generateRestConfig($service, $serviceYamlConfig, $numericEnums);
             $code = Formatter::format($code);
             yield ["src/{$version}resources/{$service->restConfigFilename}", $code];
             // Resource: client_config.json

diff --git a/src/Generation/ResourcesGenerator.php b/src/Generation/ResourcesGenerator.php
@@ -181,7 +181,7 @@ private static function restMethodDetails(ProtoCatalog $catalog, $methodOrMethod
         return AST::array($methodDetails);
     }
 
-    public static function generateRestConfig(ServiceDetails $serviceDetails, ServiceYamlConfig $serviceYamlConfig): string
+    public static function generateRestConfig(ServiceDetails $serviceDetails, ServiceYamlConfig $serviceYamlConfig, $numericEnums = false): string
     {
         $allInterfaces = static::compileRestConfigInterfaces($serviceDetails, $serviceYamlConfig);
         if ($serviceDetails->hasCustomOp) {
@@ -198,11 +198,17 @@ public static function generateRestConfig(ServiceDetails $serviceDetails, Servic
             $opInter = static::compileRestConfigInterfaces($customOpDetails, $serviceYamlConfig);
             $allInterfaces = array_merge($allInterfaces, $opInter);
         }
+
+        $config = [
+            'interfaces' => AST::array($allInterfaces)
+        ];
+
+        if ($numericEnums) {
+            $config['numericEnums'] = true;
+        }
 
         $return = AST::return(
-            AST::array([
-                'interfaces' => AST::array($allInterfaces)
-            ])
+            AST::array($config)
         );
         return "<?php\n\n{$return->toCode()};";
     }

diff --git a/src/Main.php b/src/Main.php
@@ -138,7 +138,7 @@ function showUsageAndExit()
     $descBytes = file_get_contents($opts['descriptor']);
     $package = $opts['package'];
     $outputDir = $opts['output'];
-    [$grpcServiceConfig, $gapicYaml, $serviceYaml, $transport, $generateGapicMetadata] = readOptions($opts);
+    [$grpcServiceConfig, $gapicYaml, $serviceYaml, $transport, $generateGapicMetadata, $numericEnums] = readOptions($opts);
 
     // Generate PHP code.
     $files = CodeGenerator::generateFromDescriptor(
@@ -148,7 +148,8 @@ function showUsageAndExit()
         $generateGapicMetadata,
         $grpcServiceConfig,
         $gapicYaml,
-        $serviceYaml
+        $serviceYaml,
+        $numericEnums
     );
 
     if (is_null($outputDir)) {
@@ -218,6 +219,7 @@ function readOptions($opts, $sideLoadedRootDir = null)
 
     // Flip the flag value because PHP is weird: the presence of the --metadata flag evaluates to false.
     $generateGapicMetadata =  (isset($opts['metadata']) && !$opts['metadata']);
+    $numericEnums =  (isset($opts['rest-numeric-enums']) && !$opts['rest-numeric-enums']);
 
-    return [$grpcServiceConfig, $gapicYaml, $serviceYaml, $transport, $generateGapicMetadata];
+    return [$grpcServiceConfig, $gapicYaml, $serviceYaml, $transport, $generateGapicMetadata, $numericEnums];
 }
diff --git a/tests/Tools/GeneratorUtils.php b/tests/Tools/GeneratorUtils.php
@@ -44,6 +44,7 @@ public static function generateFromProto(string $protoPath, ?string $package = n
         $grpcServiceConfigJson = ConfigLoader::loadConfig("{$protoDirName}/grpc-service-config.json");
         $gapicYaml = ConfigLoader::loadConfig("{$protoDirName}/{$baseName}_gapic.yaml");
         $serviceYaml = ConfigLoader::loadConfig("{$protoDirName}/{$baseName}_service.yaml");
+        $numericEnums = true;
 
         $licenseYear = 2022; // Avoid updating tests all the time.
         $generateGapicMetadata = true;
@@ -55,6 +56,7 @@ public static function generateFromProto(string $protoPath, ?string $package = n
             $grpcServiceConfigJson,
             $gapicYaml,
             $serviceYaml,
+            $numericEnums,
             $licenseYear
         );
         return $codeIterator;

diff --git a/tests/Unit/ProtoTests/Basic/out/src/resources/basic_rest_client_config.php b/tests/Unit/ProtoTests/Basic/out/src/resources/basic_rest_client_config.php
@@ -15,4 +15,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/...otoTests/BasicBidiStreaming/out/src/resources/basic_bidi_streaming_rest_client_config.php b/...otoTests/BasicBidiStreaming/out/src/resources/basic_bidi_streaming_rest_client_config.php
@@ -2,4 +2,5 @@
 
 return [
     'interfaces' => [],
+    'numericEnums' => true,
 ];
diff --git a/...ests/BasicClientStreaming/out/src/resources/basic_client_streaming_rest_client_config.php b/...ests/BasicClientStreaming/out/src/resources/basic_client_streaming_rest_client_config.php
@@ -2,4 +2,5 @@
 
 return [
     'interfaces' => [],
+    'numericEnums' => true,
 ];
diff --git a/...s/Unit/ProtoTests/BasicDiregapic/out/src/resources/library_service_rest_client_config.php b/...s/Unit/ProtoTests/BasicDiregapic/out/src/resources/library_service_rest_client_config.php
@@ -335,4 +335,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/tests/Unit/ProtoTests/BasicLro/out/src/resources/basic_lro_rest_client_config.php b/tests/Unit/ProtoTests/BasicLro/out/src/resources/basic_lro_rest_client_config.php
@@ -20,4 +20,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/tests/Unit/ProtoTests/BasicOneof/out/src/resources/basic_oneof_rest_client_config.php b/tests/Unit/ProtoTests/BasicOneof/out/src/resources/basic_oneof_rest_client_config.php
@@ -10,4 +10,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/...s/Unit/ProtoTests/BasicPaginated/out/src/resources/basic_paginated_rest_client_config.php b/...s/Unit/ProtoTests/BasicPaginated/out/src/resources/basic_paginated_rest_client_config.php
@@ -10,4 +10,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/...ests/BasicServerStreaming/out/src/resources/basic_server_streaming_rest_client_config.php b/...ests/BasicServerStreaming/out/src/resources/basic_server_streaming_rest_client_config.php
@@ -10,4 +10,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/.../Unit/ProtoTests/CustomLro/out/src/resources/custom_lro_operations_rest_client_config.php b/.../Unit/ProtoTests/CustomLro/out/src/resources/custom_lro_operations_rest_client_config.php
@@ -22,4 +22,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/tests/Unit/ProtoTests/CustomLro/out/src/resources/custom_lro_rest_client_config.php b/tests/Unit/ProtoTests/CustomLro/out/src/resources/custom_lro_rest_client_config.php
@@ -29,4 +29,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/.../ProtoTests/DeprecatedService/out/src/resources/deprecated_service_rest_client_config.php b/.../ProtoTests/DeprecatedService/out/src/resources/deprecated_service_rest_client_config.php
@@ -2,4 +2,5 @@
 
 return [
     'interfaces' => [],
+    'numericEnums' => true,
 ];
diff --git a/...rpcServiceConfig/out/src/resources/grpc_service_config_with_retry1_rest_client_config.php b/...rpcServiceConfig/out/src/resources/grpc_service_config_with_retry1_rest_client_config.php
@@ -25,4 +25,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/tests/Unit/ProtoTests/ResourceNames/out/src/resources/resource_names_rest_client_config.php b/tests/Unit/ProtoTests/ResourceNames/out/src/resources/resource_names_rest_client_config.php
@@ -45,4 +45,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];
diff --git a/...s/Unit/ProtoTests/RoutingHeaders/out/src/resources/routing_headers_rest_client_config.php b/...s/Unit/ProtoTests/RoutingHeaders/out/src/resources/routing_headers_rest_client_config.php
@@ -179,4 +179,5 @@
             ],
         ],
     ],
+    'numericEnums' => true,
 ];