Enhance spec-filter to support x-operation-group filtering and merge parameters across operations in same x-operation-group instead of selecting by max parameters

Signed-off-by: xil <fridalu66@gmail.com>

Author: lucy66hw

CHANGELOG.md

@@ -4,6 +4,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
 ### Added
+- Enhance spec-filter to support x-operation-group filtering and merge parameters across operations in same x-operation-group instead of selecting by max parameters ([#374](https://github.com/opensearch-project/opensearch-protobufs/pull/374))
 
 ### Changed
 

tools/proto-convert/src/Filter.ts

@@ -40,13 +40,13 @@ function traverse_and_enqueue(node: any, queue: string[], visited: Set<string>,
 export default class Filter {
   protected input: Record<string, any>
   protected output: Record<string, any>
-  protected targetPaths: string[]
+  protected targetPathsMap: Map<string, Set<string> | null>  // path -> operation-groups (null means all operations)
   protected excludedSchemas: Set<string>
   paths: Record<string, Record<string, OpenAPIV3.PathItemObject>> = {} // namespace -> path -> path_item_object
 
-  constructor(input: Record<string, any>, targetPaths: string[], excludedSchemas: Set<string> = new Set()) {
+  constructor(input: Record<string, any>, targetPathsMap: Map<string, Set<string> | null>, excludedSchemas: Set<string> = new Set()) {
     this.input = input;
-    this.targetPaths = targetPaths;
+    this.targetPathsMap = targetPathsMap;
     this.excludedSchemas = excludedSchemas;
     if (this.excludedSchemas.size > 0) {
       logger.info(`Loaded ${this.excludedSchemas.size} excluded schemas: ${Array.from(this.excludedSchemas).join(', ')}`);
@@ -67,14 +67,32 @@ export default class Filter {
 
   filter(): OpenAPIV3.Document {
     this.output.info = this.input.info;
-    for (const p of this.targetPaths) {
-      if (this.input.paths[p] === undefined) {
-        logger.error(`Path not found in spec: ${p}`);
+
+    for (const [targetPath, targetGroups] of this.targetPathsMap) {
+      if (this.input.paths[targetPath] === undefined) {
+        logger.error(`Path not found in spec: ${targetPath}`);
         continue;
       }
-      this.output.paths[p] = this.input.paths[p];
+
+      const pathItem = this.input.paths[targetPath];
+
+      if (targetGroups === null) {
+        this.output.paths[targetPath] = pathItem;
+      } else {
+        const filteredPathItem: any = {};
+        for (const method of ['get', 'post', 'put', 'delete', 'head'] as const) {
+          const operation = pathItem?.[method];
+          if (operation && operation['x-operation-group'] && targetGroups.has(operation['x-operation-group'])) {
+            filteredPathItem[method] = operation;
+          }
+        }
+        if (Object.keys(filteredPathItem).length > 0) {
+          this.output.paths[targetPath] = filteredPathItem;
+        }
+      }
     }
-    this.filter_by_max_parameters(this.output.paths as OpenAPIV3.PathsObject);
+
+    this.mergeOperationsByGroup(this.output.paths as OpenAPIV3.PathsObject);
     const queue: string[] = [];
     const visited: Set<string> = new Set();
 
@@ -101,65 +119,98 @@ export default class Filter {
     return this.output as OpenAPIV3.Document;
   }
 
-  filter_by_max_parameters(paths: OpenAPIV3.PathsObject): void {
+  /**
+   * Merges operations with the same x-operation-group into a single operation.
+   * - Validates that requestBody and responses are identical across paths
+   * - Merges all parameters from different paths into a union
+   */
+  mergeOperationsByGroup(paths: OpenAPIV3.PathsObject): void {
     const new_paths: OpenAPIV3.PathsObject = {};
-    let operation_map = new Map<string, Array<Record<string, OpenAPIV3.PathItemObject>>>();
+
+    // Group operations by x-operation-group
+    type OperationInfo = {
+      path: string;
+      method: string;
+      operation: OpenAPIV3.OperationObject;
+    };
+    const operationsByGroup = new Map<string, OperationInfo[]>();
+
     for (const path in paths) {
       const path_item = paths[path];
       if (!path_item) continue;
       for (const method of Object.keys(path_item) as Array<keyof OpenAPIV3.PathItemObject>) {
         const operation = path_item[method];
         if (operation != null && typeof operation === 'object' && 'x-operation-group' in operation) {
           const group: string = operation['x-operation-group'] as string;
-          if (!operation_map.get(group)) {
-            operation_map.set(group, []);
+          if (!operationsByGroup.has(group)) {
+            operationsByGroup.set(group, []);
           }
-          const group_map: Record<string, OpenAPIV3.PathItemObject> = {
-            [path]: {
-              [method]: operation as OpenAPIV3.OperationObject,
-            },
-          };
-          operation_map.get(group)?.push(group_map);
+          operationsByGroup.get(group)!.push({
+            path,
+            method,
+            operation: operation as OpenAPIV3.OperationObject
+          });
         }
       }
     }
 
+    for (const [group, operations] of operationsByGroup.entries()) {
+      if (operations.length === 0) continue;
 
-    for (const operations of operation_map.values()) {
-      let max_parameters = -1;
-      let max_path_item: OpenAPIV3.PathItemObject | null = null;
-      let max_path = '';
-      for (const op of operations) {
-        for (const [path, path_item] of Object.entries(op)) {
-          for (const operation of Object.values(path_item)) {
-            if (operation != null && typeof operation === 'object' && Array.isArray((operation as OpenAPIV3.OperationObject).parameters)) {
-              const param_count = (operation as OpenAPIV3.OperationObject).parameters?.length??0;
-              if (param_count > max_parameters) {
-                max_parameters = param_count;
-                max_path = path;
-                max_path_item = path_item;
-              }
-            }
-          }
-        }
-      }
-      if ((max_path_item?.get?.operationId) != null) {
-        max_path_item.get.operationId = max_path_item.get.operationId.replace(/\.\d+$/, '');
-        new_paths[max_path] = { ...new_paths[max_path], get: max_path_item.get };
-      } else if ((max_path_item?.post?.operationId != null))  {
-        max_path_item.post.operationId = max_path_item.post.operationId.replace(/\.\d+$/, '');
-        new_paths[max_path] = { ...new_paths[max_path], post: max_path_item.post };
-      } else if ((max_path_item?.put?.operationId != null)) {
-        max_path_item.put.operationId = max_path_item.put.operationId.replace(/\.\d+$/, '');
-        new_paths[max_path] = { ...new_paths[max_path], put: max_path_item.put };
-      } else if ((max_path_item?.delete?.operationId != null)) {
-        max_path_item.delete.operationId = max_path_item.delete.operationId.replace(/\.\d+$/, '');
-        new_paths[max_path] = { ...new_paths[max_path], delete: max_path_item.delete };
-      } else if ((max_path_item?.head?.operationId != null)) {
-        max_path_item.head.operationId = max_path_item.head.operationId.replace(/\.\d+$/, '');
-        new_paths[max_path] = { ...new_paths[max_path], head: max_path_item.head };
+      // 1. Validate requestBody and responses are consistent before merging
+      this.validateConsistency(group, operations);
+
+      // 2. Merge parameters from all operations
+      const mergedParams = this.mergeParameters(operations);
+
+      // 3. Create merged operation
+      const firstOp = operations[0];
+      const mergedOperation: OpenAPIV3.OperationObject = {
+        ...firstOp.operation,
+        operationId: firstOp.operation.operationId?.replace(/\.\d+$/, ''),
+        parameters: mergedParams
+      };
+
+      if (!new_paths[firstOp.path]) {
+        new_paths[firstOp.path] = {};
       }
+      (new_paths[firstOp.path] as any)[firstOp.method] = mergedOperation;
     }
+
     this.output.paths = new_paths;
   }
+
+  private validateConsistency(group: string, operations: { path: string; operation: OpenAPIV3.OperationObject }[]): void {
+    // Check requestBody consistency
+    const requestBodies = operations
+      .map(op => JSON.stringify(op.operation.requestBody))
+      .filter(rb => rb !== 'undefined');
+    const uniqueRequestBodies = new Set(requestBodies);
+    if (uniqueRequestBodies.size > 1) {
+      throw new Error(`Operation group '${group}' has inconsistent requestBody across paths: ${operations.map(op => op.path).join(', ')}`);
+    }
+
+    // Check responses consistency
+    const responses = operations.map(op => JSON.stringify(op.operation.responses));
+    const uniqueResponses = new Set(responses);
+    if (uniqueResponses.size > 1) {
+      throw new Error(`Operation group '${group}' has inconsistent responses across paths: ${operations.map(op => op.path).join(', ')}`);
+    }
+  }
+
+  private mergeParameters(operations: { operation: OpenAPIV3.OperationObject }[]): (OpenAPIV3.ReferenceObject | OpenAPIV3.ParameterObject)[] {
+    const allParams = new Map<string, OpenAPIV3.ReferenceObject | OpenAPIV3.ParameterObject>();
+    for (const op of operations) {
+      const params = op.operation.parameters ?? [];
+      for (const param of params) {
+        const key = '$ref' in param
+          ? param.$ref
+          : `${(param as OpenAPIV3.ParameterObject).name}:${(param as OpenAPIV3.ParameterObject).in}`;
+        if (!allParams.has(key)) {
+          allParams.set(key, param);
+        }
+      }
+    }
+    return Array.from(allParams.values());
+  }
 }

tools/proto-convert/src/PreProcessing.ts

@@ -1,5 +1,8 @@
 import { Command, Option } from '@commander-js/extra-typings';
-import { read_yaml, write_yaml } from './utils/helper';
+import { read_yaml, write_yaml, parsePathsConfig } from './utils/helper';
+
+// Path config type: path -> { x-operation-group: string[] }
+type PathConfig = Record<string, { 'x-operation-group'?: string[] } | null>;
 import Filter from './Filter';
 import { Sanitizer } from './Sanitizer';
 import logger from './utils/logger';
@@ -10,21 +13,17 @@ import {GlobalParameterConsolidator} from "./GlobalParamWrapper";
 import {OpenSearchVersionExtractor} from "./OpenSearchVersionExtractor";
 
 // Load config from spec-filter.yaml
-const config = read_yaml<{ paths?: string[]; excluded_schemas?: string[] }>(
+const config = read_yaml<{ paths?: PathConfig; excluded_schemas?: string[] }>(
   path.join(__dirname, 'config', 'spec-filter.yaml')
 );
-const target_paths = config.paths ?? ['/_search'];
+
+const target_paths_map = parsePathsConfig(config.paths);
 const excluded_schemas = new Set(config.excluded_schemas ?? []);
 
 const command = new Command()
   .description('Preprocess an OpenAPI spec by filtering for specific paths and then sanitizing it.')
   .addOption(new Option('-i, --input <path>', 'input YAML file').default((path.resolve(__dirname, '../../../opensearch-openapi.yaml'))))
   .addOption(new Option('-o, --output <path>', 'output YAML file').default((path.resolve(__dirname, '../../../build/processed-opensearch-openapi.yaml'))))
-  .addOption(
-    new Option('-p, --filtered_path <paths>', 'the paths to keep (comma-separated, e.g., /_search,)')
-      .argParser((val: string) => val.split(',').map(s => s.trim()))
-      .default(target_paths)
-  )
   .addOption(new Option('--verbose', 'show merge details').default(false))
   .addOption(new Option('--opensearch-version <version>', 'current OpenSearch version for deprecation removal').default('3.4'))
   .allowExcessArguments(false)
@@ -34,17 +33,17 @@ const command = new Command()
 type PreprocessingOpts = {
   input: string;
   output: string;
-  filtered_path: string[];
   verbose: boolean;
   opensearchVersion: string;
 };
 
 const opts = command.opts() as PreprocessingOpts;
 
 try {
-  logger.info(`PreProcessing ${opts.filtered_path.join(', ')} into ${opts.output} ...`)
+  const pathsList = Array.from(target_paths_map.keys());
+  logger.info(`PreProcessing ${pathsList.join(', ')} into ${opts.output} ...`)
   const original_spec = read_yaml(opts.input)
-  const filtered_spec = new Filter(original_spec, opts.filtered_path, excluded_schemas).filter();
+  const filtered_spec = new Filter(original_spec, target_paths_map, excluded_schemas).filter();
   const version_processed_spec = new OpenSearchVersionExtractor(filtered_spec).process(opts.opensearchVersion);
   const sanitized_spec = new Sanitizer(version_processed_spec).sanitize();
   const consolidated_spec = new GlobalParameterConsolidator(sanitized_spec).consolidate();

tools/proto-convert/src/config/protobuf-schema-template/partial_header.mustache

@@ -4,9 +4,7 @@
 syntax = "proto3";
 package org.opensearch.protobufs;
 
-import "google/protobuf/struct.proto";
-
 option go_package = "github.com/opensearch-project/opensearch-protobufs/go/opensearchpb";
 option java_multiple_files = true;
 option java_outer_classname = "CommonProto";
-option java_package = "org.opensearch.protobufs";
+option java_package = "org.opensearch.protobufs";

tools/proto-convert/src/config/spec-filter.yaml

@@ -1,7 +1,17 @@
 # Target API paths to include in proto generation
+# Format:
+#   path:
+#     x-operation-group:    # optional, list of operation groups to include
+#       - group1
+#       - group2
+# If x-operation-group is not specified, all operations on that path are included
 paths:
-  - /{index}/_bulk
-  - /{index}/_search
+  /{index}/_bulk:
+    x-operation-group:
+    - bulk
+  /{index}/_search:
+    x-operation-group:
+    - search
 
 # Schemas to exclude from proto generation
 # These schemas and their nested dependencies will not be included

tools/proto-convert/src/utils/helper.ts

@@ -198,6 +198,10 @@ export function remove_unused(spec: OpenAPIV3.Document): void {
             !_.includes(remaining, obj.additionalProperties.items.$ref)) {
             return true;
         }
+        // Case 4: Object has empty properties (properties: {})
+        if (obj.properties && _.isObject(obj.properties) && _.isEmpty(obj.properties) && obj.type !== 'object') {
+            return true;
+        }
         return false;
     });
 }
@@ -212,3 +216,21 @@ export function is_simple_ref(schema: any): boolean {
     const keys = Object.keys(schema);
     return keys.length === 1 && '$ref' in schema;
 }
+
+/**
+ * Convert paths config to Map<path, Set<operation-groups>>
+ * @param paths - The path configuration from spec-filter.yaml
+ * @returns Map where key is path and value is Set of operation groups (null means all operations)
+ */
+export function parsePathsConfig(paths: Record<string, { 'x-operation-group'?: string[] } | null> | undefined): Map<string, Set<string> | null> {
+    const result = new Map<string, Set<string> | null>();
+    if (!paths) {
+        result.set('/_search', null);
+        return result;
+    }
+    for (const [p, config] of Object.entries(paths)) {
+        const groups = config?.['x-operation-group'];
+        result.set(p, groups && groups.length > 0 ? new Set(groups) : null);
+    }
+    return result;
+}