feat: add API definition PATCH for v4 APIs#15830
feat: add API definition PATCH for v4 APIs#15830ankita-gupta21 wants to merge 4 commits intomasterfrom
Conversation
2b75a6e to
83a2ce9
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new feature by providing a PATCH endpoint for V4 API definitions. This allows for more efficient and granular updates to API configurations, aligning with modern API management practices. The implementation includes robust error handling, concurrency control, and permission checks, ensuring that API definitions can be modified safely and precisely, particularly for V4 HTTP Proxy APIs. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new REST endpoint, PATCH /apis/{apiId}/definition, to enable partial updates of V4 HTTP Proxy API definitions using JSON Patch (RFC 6902). The changes include a new ApiDefinitionResource to handle these operations, a new mapping method in ApiMapper to convert patched API exports to UpdateApiV4 objects, and integration of this resource into the existing ApiResource. Comprehensive unit tests have been added for the new endpoint, covering scenarios such as dry runs, optimistic locking with If-Match headers, permission checks, and validation for V4 HTTP Proxy API types. Additionally, the test configuration has been updated to provide the JsonPatchService bean.
83a2ce9 to
50f7c16
Compare
vikrantgravitee
left a comment
There was a problem hiding this comment.
We could have Patch method in ApiResource
cb375b3 to
dc49bd7
Compare
There was a problem hiding this comment.
Pull request overview
Adds support in Management API v2 for partially updating V4 HTTP Proxy API definitions via a JSON-patch-like mechanism on the exported definition document, with optional persistence (dryRun) and revision control (If-Match).
Changes:
- Add
PATCH /apis/{apiId}/definitionendpoint for V4 HTTP Proxy APIs, integrating export → patch → map-to-update → update → re-export flow. - Introduce REST integration tests covering dry-run, persistence,
If-Matchhandling, permission failures, and unsupported API kinds. - Wire
JsonPatchServiceinto the Management v2 REST test Spring context.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.../spring/ResourceContextConfiguration.java |
Adds a JsonPatchService bean for REST integration tests. |
.../resource/api/ApiResource_PatchApiDefinitionTest.java |
New integration tests validating PATCH definition behavior (200/204/400/403/412). |
.../resource/api/ApiResource.java |
Implements PATCH /definition for V4 HTTP Proxy API definitions, including dry-run and ETag support. |
.../mapper/ApiMapper.java |
Adds mapping from ApiV4 to UpdateApiV4 to reuse existing V4 update path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-v2-rest/src/main/java/io/gravitee/rest/api/management/v2/rest/resource/api/ApiResource.java
Outdated
Show resolved
Hide resolved
...-v2-rest/src/main/java/io/gravitee/rest/api/management/v2/rest/resource/api/ApiResource.java
Outdated
Show resolved
Hide resolved
cdd6f8c to
76f775d
Compare
|
76f775d to
007006b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case PatchApiDefinitionUseCase.Result.DryRun(String json) -> Response.ok( | ||
| ImportExportApiMapper.INSTANCE.definitionToExportApiV4(json) | ||
| ).build(); |
There was a problem hiding this comment.
The PR description/examples state that dryRun=true returns the patched JSON as a raw string, but the implementation returns an ExportApiV4 JSON object (and the OpenAPI spec also describes ExportApiV4). Please align the PR description/examples with the actual response shape (or adjust the implementation/spec if the raw string response is required).
| responses: | ||
| "200": | ||
| description: |- | ||
| Patched API definition in `ExportApiV4` form (same shape as `GET .../_export/definition`). | ||
|
|
||
| With `dryRun=true`, the body is a preview only and is not persisted; `ETag` and `Last-Modified` are omitted, like the export endpoint. | ||
|
|
||
| With `dryRun=false`, the API is updated and `ETag` / `Last-Modified` reflect the stored API after persist. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ExportApiV4" | ||
| "204": | ||
| description: No change applied (for example JSON Patch `test` operation did not match) | ||
| default: | ||
| $ref: "#/components/responses/Error" |
There was a problem hiding this comment.
The description mentions If-Match and ETag/Last-Modified behavior, but the OpenAPI operation does not define the optional If-Match request header nor the ETag/Last-Modified response headers for the persisted (non-dry-run) case. Please add these headers to the operation so generated clients and API docs reflect the concurrency semantics and caching metadata.
| } | ||
|
|
||
| var patchOutput = patchApiDefinitionUseCase.execute(new PatchApiDefinitionUseCase.Input(apiId, getAuditInfo(), patches, dryRun)); | ||
|
|
There was a problem hiding this comment.
If-Match is evaluated against currentEntity.getUpdatedAt(), but the patch is applied to an export fetched separately inside PatchApiDefinitionUseCase. If the API is updated between these two reads, the request can pass the precondition check yet apply the patch on a newer (or different) revision than the one validated by the ETag, weakening concurrency control. Consider evaluating If-Match against the same revision used to build the export (e.g., export definition’s api.updatedAt) and/or asserting that currentEntity.updatedAt matches the exported definition’s updatedAt before applying/persisting the patch.
| // Re-check concurrency after computing the patch result but before applying it | |
| final GenericApiEntity latestEntity = getGenericApiEntityById(apiId, false, false, false, false); | |
| if (!latestEntity.getUpdatedAt().equals(currentEntity.getUpdatedAt())) { | |
| // The API has been modified since the initial If-Match validation; reject with 412 | |
| return Response | |
| .status(Response.Status.PRECONDITION_FAILED) | |
| .tag(Long.toString(latestEntity.getUpdatedAt().getTime())) | |
| .lastModified(latestEntity.getUpdatedAt()) | |
| .build(); | |
| } |
| description: Gravitee JSON Patch entry (JSON Pointer path and operation). | ||
| required: | ||
| - jsonPath | ||
| properties: | ||
| jsonPath: | ||
| type: string | ||
| description: JSON Pointer path within the exported definition document. |
There was a problem hiding this comment.
The schema/documentation calls jsonPath a “JSON Pointer path”, but the implementation uses Jayway JsonPath expressions (e.g., $.api.name). This is likely to confuse API consumers expecting RFC 6901 pointers. Please update the description text to explicitly say JSONPath (Jayway) syntax and, if relevant, clarify that this is Gravitee’s custom patch format (not RFC 6902 path/op).
| description: Gravitee JSON Patch entry (JSON Pointer path and operation). | |
| required: | |
| - jsonPath | |
| properties: | |
| jsonPath: | |
| type: string | |
| description: JSON Pointer path within the exported definition document. | |
| description: Gravitee JSON Patch entry using JSONPath (Jayway) expressions for jsonPath (custom format, not RFC 6902/JSON Pointer). | |
| required: | |
| - jsonPath | |
| properties: | |
| jsonPath: | |
| type: string | |
| description: JSONPath (Jayway) expression within the exported definition document. |
| description: |- | ||
| Apply JSON Patch operations to the exported API definition for a V4 HTTP Proxy API. | ||
|
|
||
| The document patched is the same shape as returned by `GET .../_export/definition`. When `dryRun` is false, the API is updated using the same rules as `PUT .../apis/{apiId}` for a V4 API (including gateway-definition permission constraints on listeners). | ||
|
|
||
| User must have API_DEFINITION[UPDATE] or API_GATEWAY_DEFINITION[UPDATE] permissions. |
There was a problem hiding this comment.
The endpoint persists changes by mapping and saving only the api object (envelope fields like plans/members/pages are ignored), but the OpenAPI description currently reads as if the whole exported definition document is updated when dryRun=false. To avoid misleading clients, please document that only api is applied on persist and that patching other envelope sections is preview-only / ignored for persistence.
There was a problem hiding this comment.
Not right place to have usecase
007006b to
795895b
Compare



Issue`
`
https://gravitee.atlassian.net/browse/APIM-XXX
Description
Adds JSON Patch (RFC 6902) support for V4 HTTP Proxy APIs on
PATCH /management/v2/organizations/{orgId}/environments/{envId}/apis/{apiId}/definition, aligned with the same export shape as GET .../apis/{apiId}/_export/definition and the same update semantics as PUT .../apis/{apiId} for the api payload.
Additional context
Some Example:
Success – dry run (200, body is patched JSON string)
No persistence; If-Match optional.
Success – persist (200, full export JSON + new ETag)
Use a current ETag from GET .../apis/{apiId} (or last PUT/PATCH response). Quotes around the tag value are common.
ETAG='"1700000000000"' # example: from GET /apis/{apiId} → ETag headerCheck ETag in response headers and body in /tmp/patch-body.json.
Failure – precondition failed (412)
Wrong or stale If-Match:
Expected: 412.
Failure – unsupported API kind (400)
Use an API that is not V4 HTTP Proxy (e.g. Native V4). Same payload:
Expected: 400 with message about V4 HTTP Proxy only.
“Failure” – JSON Patch test op mismatch (204, no body)
Export must contain api.name matching reality; TEST uses a wrong expected value:
Screen.Recording.2026-03-22.at.2.01.59.PM.mov