fatmanverse commented on PR #12922: URL: https://github.com/apache/seatunnel/pull/12922#issuecomment-6492653294
Thanks @SEZ9 for the thorough review. All eight points are addressed in 98a2be8: 1. **All-or-nothing `/submit-jobs` (Medium).** `JobInfoService#submitJobs` is now two-phase: it first builds and validates the request parameters of every entry (savepoint/`jobId`, dry-run, `validateCheckpointRestoreRequest`), and only then submits the jobs. `JobInfoServiceSubmitJobsTest#testInvalidLaterEntryRejectsTheWholeRequest` sends a valid first entry and an invalid second one and asserts nothing was submitted, for `isStartWithSavePoint` without `jobId`, `restoreMode: "BOGUS"`, `restoreMode: "SAVEPOINT"` without a source job, a nested value and `"jobId": 1.5`. 2. **Blank `jobId`.** `checkStartWithSavePoint` uses `StringUtils.isBlank`, so `"jobId": ""` (or blank) with `isStartWithSavePoint: true` gets `Please provide jobId when start with save point.` This also applies to v1 `/submit-job?jobId=&isStartWithSavePoint=true`, which shares the helper. 3. **Non-integral numbers.** `toStringParams` rejects a number that is not an integer within the `long` range (`1e3`, `1.5`, `9223372036854775808`) with `'params' number values must be integers within the long range`, before anything is submitted. `Long.MAX_VALUE` is still accepted. 4. **`null` and empty string.** `RestUtilSubmitJobsParamsTest#testNullValueIsSkippedAndEmptyStringIsKept` pins `"jobId": null` (key present, value `null`, no `"null"` string) and `"jobName": ""`. 5. **Docs.** The `/submit-jobs` tables in en/zh, v1/v2 now type `jobId` as `string | number` and `isStartWithSavePoint` as `string | boolean`, and include the `restoreMode` and `restoreSourceJobId` rows copied from `/submit-job` (v1 reaches the same `JobInfoService#submitJobs`). The paragraph below the tables and the incompatible-changes entry (en/zh) also mention the rejected numbers and the all-or-nothing validation. 6. **End-to-end nested value and ordering.** Covered by the test in point 1, which includes a nested value in the second entry. 7. **Log forging.** The nested-value message no longer interpolates the client-supplied key: it is now `'params' values must be a string, number or boolean, but one was a JSON object|array`. The test uses a key containing a newline and asserts the exact message. 8. **Overload.** The `Map` overload is renamed to `buildRequestParamsFromJson(Map, Map)`; its only callers are `JobInfoService` and the test helper. On JDK 17, `./mvnw -pl seatunnel-engine/seatunnel-engine-server test` passes: 708 tests, 0 failures (5 skipped), including `RestUtilSubmitJobsParamsTest` (9) and `JobInfoServiceSubmitJobsTest` (4). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
