Skip to content

Commit 011902a

Browse files
lucy66hwlucy66hw
authored
Refactor postprocessing: consolidate proto writer and auto-detect cleanup roots from RPC (opensearch-project#347)
* parsing service definition to support auto-detect root messages for cleanup Signed-off-by: xil <fridalu66@gmail.com> * simplify Signed-off-by: xil <fridalu66@gmail.com> ignore CLI code Signed-off-by: xil <fridalu66@gmail.com> --------- Signed-off-by: xil <fridalu66@gmail.com>
1 parent 3e0e12d commit 011902a

14 files changed

Lines changed: 1107 additions & 180 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
55
## [Unreleased]
66
### Added
77
- Add unit test workflow with coverage reporting ([#346](https://github.com/opensearch-project/opensearch-protobufs/pull/346))
8+
- parsing service definition to support auto-detect root messages for cleanup ([#347](https://github.com/opensearch-project/opensearch-protobufs/pull/347))
89

910
### Changed
1011

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
"scripts": {
88
"preprocessing": "ts-node tools/proto-convert/src/PreProcessing.ts",
99
"backward-compat": "ts-node tools/proto-convert/src/postprocessing/BackwardCompatibleWriter.ts",
10-
"cleanup-common": "ts-node tools/proto-convert/src/postprocessing/CleanupUnusedMessages.ts -i protos/schemas/common.proto",
11-
"postprocessing": "npm run backward-compat && npm run cleanup-common",
10+
"cleanup-unused": "ts-node tools/proto-convert/src/postprocessing/CleanupUnusedMessages.ts -i protos/schemas/common.proto",
11+
"postprocessing": "npm run backward-compat && npm run cleanup-unused",
1212
"test": "npx jest --no-watchman"
1313
},
1414
"dependencies": {

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,18 @@ import "{{{.}}}.proto";
66
{{/import}}
77
{{/imports}}
88

9+
service {{classname}} {
10+
{{#operations}}
11+
{{#operation}}
12+
{{#description}}
13+
// {{{.}}}
14+
{{/description}}
15+
rpc {{operationId}} ({{#hasParams}}{{operationId}}Request{{/hasParams}}{{^hasParams}}google.protobuf.Empty{{/hasParams}}) returns ({{#vendorExtensions.x-grpc-response}}{{.}}{{/vendorExtensions.x-grpc-response}}{{^vendorExtensions.x-grpc-response}}{{operationId}}Response{{/vendorExtensions.x-grpc-response}});
16+
17+
{{/operation}}
18+
{{/operations}}
19+
}
20+
921
{{#operations}}
1022
{{#operation}}
1123
{{#hasParams}}

‎tools/proto-convert/src/postprocessing/BackwardCompatibleWriter.ts‎

Lines changed: 62 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { existsSync, readFileSync, writeFileSync } from 'fs';
2-
import { join } from 'path';
1+
import { existsSync } from 'fs';
32
import { Command, Option } from '@commander-js/extra-typings';
43
import {
54
ProtoMessage,
@@ -8,39 +7,9 @@ import {
87
} from './types';
98
import { parseProtoFile } from './parser';
109
import { mergeMessage, mergeEnum } from './CompatibilityMerger';
11-
import { generateMessage, generateEnum } from './writer';
10+
import { writeProtoFile, CUSTOM_MESSAGE_NAMES, CUSTOM_ENUM_NAMES } from './writer';
1211
import logger from '../utils/logger';
1312

14-
const TEMPLATE_DIR = join(__dirname, '../config/protobuf-schema-template');
15-
16-
// Load fixed header and custom messages from templates
17-
const PROTO_HEADER = readFileSync(join(TEMPLATE_DIR, 'partial_header.mustache'), 'utf-8');
18-
const CUSTOM_MESSAGES = readFileSync(join(TEMPLATE_DIR, 'custom_message.mustache'), 'utf-8');
19-
20-
// ==================== CLI ====================
21-
22-
const command = new Command()
23-
.description('Merge incoming proto files into existing proto while maintaining backward compatibility.')
24-
.addOption(new Option('-e, --existing <path>', 'existing proto file (source of truth)').default('protos/schemas/common.proto'))
25-
.addOption(new Option('-i, --incoming <paths>', 'incoming proto files (comma-separated)')
26-
.argParser((val: string) => val.split(',').map(s => s.trim()))
27-
.default(['protos/generated/models/aggregated_models.proto', 'protos/generated/services/default_service.proto']))
28-
.addOption(new Option('-o, --output <path>', 'output proto file').default('protos/schemas/common.proto'))
29-
.allowExcessArguments(false)
30-
.parse();
31-
32-
type BackwardCompatOpts = {
33-
existing: string;
34-
incoming: string[];
35-
output: string;
36-
};
37-
38-
const opts = command.opts() as BackwardCompatOpts;
39-
40-
// Messages defined in custom_message.mustache - skip and use template instead
41-
const CUSTOM_MESSAGE_NAMES = new Set(['ObjectMap', 'GeneralNumber']);
42-
const CUSTOM_ENUM_NAMES = new Set(['NullValue']);
43-
4413
export class BackwardCompatibleWriter {
4514
private existingMessages: ProtoMessage[];
4615
private existingEnums: ProtoEnum[];
@@ -78,63 +47,55 @@ export class BackwardCompatibleWriter {
7847
}
7948

8049
process(): void {
81-
const outputParts: string[] = [];
82-
83-
// Use fixed header from template
84-
outputParts.push(PROTO_HEADER.trim());
50+
const finalMessages: ProtoMessage[] = [];
51+
const finalEnums: ProtoEnum[] = [];
8552

86-
// Process messages
53+
// Process existing messages (merge with incoming if present)
8754
for (const existingMsg of this.existingMessages) {
8855
if (CUSTOM_MESSAGE_NAMES.has(existingMsg.name)) {
8956
this.incomingMessageMap.delete(existingMsg.name);
9057
continue;
9158
}
9259

9360
const incomingMsg = this.incomingMessageMap.get(existingMsg.name);
94-
9561
if (incomingMsg) {
96-
const mergedMsg = mergeMessage(existingMsg, incomingMsg, this.errors);
97-
outputParts.push(generateMessage(mergedMsg));
62+
finalMessages.push(mergeMessage(existingMsg, incomingMsg, this.errors));
9863
this.incomingMessageMap.delete(existingMsg.name);
9964
} else {
100-
outputParts.push(generateMessage(existingMsg));
65+
finalMessages.push(existingMsg);
10166
}
10267
}
10368

104-
// Process enums
69+
// Process existing enums (merge with incoming if present)
10570
for (const existingEnum of this.existingEnums) {
10671
if (CUSTOM_ENUM_NAMES.has(existingEnum.name)) {
10772
this.incomingEnumMap.delete(existingEnum.name);
10873
continue;
10974
}
11075

11176
const incomingEnum = this.incomingEnumMap.get(existingEnum.name);
112-
11377
if (incomingEnum) {
114-
const mergedEnum = mergeEnum(existingEnum, incomingEnum);
115-
outputParts.push(generateEnum(mergedEnum));
78+
finalEnums.push(mergeEnum(existingEnum, incomingEnum));
11679
this.incomingEnumMap.delete(existingEnum.name);
11780
} else {
118-
outputParts.push(generateEnum(existingEnum));
81+
finalEnums.push(existingEnum);
11982
}
12083
}
12184

122-
// Append new messages from incoming proto files
85+
// Add new messages from incoming (not in existing)
12386
for (const [, msg] of this.incomingMessageMap) {
124-
outputParts.push('');
125-
outputParts.push(generateMessage(msg));
87+
if (!CUSTOM_MESSAGE_NAMES.has(msg.name)) {
88+
finalMessages.push(msg);
89+
}
12690
}
12791

128-
// Append new enums from incoming proto files
92+
// Add new enums from incoming (not in existing)
12993
for (const [, protoEnum] of this.incomingEnumMap) {
130-
outputParts.push('');
131-
outputParts.push(generateEnum(protoEnum));
94+
if (!CUSTOM_ENUM_NAMES.has(protoEnum.name)) {
95+
finalEnums.push(protoEnum);
96+
}
13297
}
13398

134-
// Append custom messages from template (ObjectMap, GeneralNumber, NullValue)
135-
outputParts.push('');
136-
outputParts.push(CUSTOM_MESSAGES.trim());
137-
13899
// Check for errors before writing
139100
if (this.errors.length > 0) {
140101
logger.error('Backward compatibility errors:');
@@ -146,36 +107,56 @@ export class BackwardCompatibleWriter {
146107
);
147108
}
148109

149-
writeFileSync(this.outputPath, outputParts.join('\n'));
110+
// Write output using shared function
111+
writeProtoFile(finalMessages, finalEnums, this.outputPath);
150112
logger.info(`Updated: ${this.outputPath}`);
151113
}
152114
}
153115

154-
// ==================== RUN ====================
155-
156-
if (!existsSync(opts.existing)) {
157-
logger.error(`Existing file not found: ${opts.existing}`);
158-
process.exit(1);
159-
}
116+
// ==================== CLI ====================
160117

161-
const existingIncoming = opts.incoming.filter(p => existsSync(p));
162-
if (existingIncoming.length === 0) {
163-
logger.error(`No incoming proto files found.`);
164-
process.exit(1);
165-
}
118+
/* istanbul ignore next -- CLI entry point */
119+
if (require.main === module) {
120+
const command = new Command()
121+
.description('Merge incoming proto files into existing proto while maintaining backward compatibility.')
122+
.addOption(new Option('-e, --existing <path>', 'existing proto file (source of truth)').default('protos/schemas/common.proto'))
123+
.addOption(new Option('-i, --incoming <paths>', 'incoming proto files (comma-separated)')
124+
.argParser((val: string) => val.split(',').map(s => s.trim()))
125+
.default(['protos/generated/models/aggregated_models.proto', 'protos/generated/services/default_service.proto']))
126+
.addOption(new Option('-o, --output <path>', 'output proto file').default('protos/schemas/common.proto'))
127+
.allowExcessArguments(false)
128+
.parse();
129+
130+
type BackwardCompatOpts = {
131+
existing: string;
132+
incoming: string[];
133+
output: string;
134+
};
135+
136+
const opts = command.opts() as BackwardCompatOpts;
137+
138+
if (!existsSync(opts.existing)) {
139+
logger.error(`Existing file not found: ${opts.existing}`);
140+
process.exit(1);
141+
}
166142

167-
try {
168-
const writer = new BackwardCompatibleWriter(
169-
opts.existing,
170-
opts.incoming,
171-
opts.output
172-
);
173-
writer.process();
174-
} catch (error) {
175-
if (error instanceof BackwardCompatibilityError) {
143+
const existingIncoming = opts.incoming.filter(p => existsSync(p));
144+
if (existingIncoming.length === 0) {
145+
logger.error(`No incoming proto files found.`);
176146
process.exit(1);
177147
}
178-
throw error;
179-
}
180148

181-
export { BackwardCompatibilityError };
149+
try {
150+
const writer = new BackwardCompatibleWriter(
151+
opts.existing,
152+
opts.incoming,
153+
opts.output
154+
);
155+
writer.process();
156+
} catch (error) {
157+
if (error instanceof BackwardCompatibilityError) {
158+
process.exit(1);
159+
}
160+
throw error;
161+
}
162+
}

0 commit comments

Comments
 (0)