Skip to content

Commit

Permalink
Address PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mkistler committed Feb 16, 2024
1 parent 1a4fe98 commit 56534bc
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 192 deletions.
120 changes: 13 additions & 107 deletions aep/general/0135/aep.md.j2
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ In REST APIs, it is customary to make a `DELETE` request to a resource's URI
that resource.

Resource-oriented design ([AEP-121][aep-121]) honors this pattern through the `Delete`
method. These operations accept the URI representing that resource and usually return
method. This method accepts the URI representing that resource and usually returns
an empty response.

## Guidance
Expand All @@ -32,12 +32,9 @@ Delete methods are specified using the following pattern:
- The request **must not** require any fields in the query string. The request
**should not** include optional fields in the query string unless described
in another AEP.
- Delete operations **must** return `204 No Content` with no response body.
- Exception: If the resource is soft deleted ([AIP-164][aep-164]), in which case the
operation **must** return `200 OK` and the resource itself, without any
additional wrapping.
- If the API is operating on the [Management Plane][], the operation should have
[strong consistency][]: the completion of a delete operation **must** mean
- Delete methods **must** return `204 No Content` with no response body.
- If the API is operating on the [Management Plane][], the method should have
[strong consistency][]: the completion of a delete method **must** mean
that the existence of the resource has reached a steady-state and reading
resource state returns a consistent response.

Expand Down Expand Up @@ -73,8 +70,8 @@ rpc DeleteBook(DeleteBookRequest) returns (google.protobuf.Empty) {
- There **should** be exactly one `google.api.method_signature` annotation,
with a value of `"name"`. If an etag or force field are used, they **may** be
included in the signature.
- If the API is operating on the [Management Plane][], the operation should have
[strong consistency][]: the completion of a delete operation **must** mean
- If the API is operating on the [Management Plane][], the method should have
[strong consistency][]: the completion of a delete method **must** mean
that the existence of the resource has reached a steady-state and reading
resource state returns a consistent response.

Expand All @@ -87,7 +84,7 @@ message DeleteBookRequest {
string name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference) = {
type: "library.googleapis.com/Book"
type: "library.example.com/Book"
}];
}
```
Expand Down Expand Up @@ -120,11 +117,9 @@ operation instead. See [AEP-151][aep-151] for full details on long-running opera

- The response status code should be `202 Accepted` if the request was
accepted for later processing. When the request is processed it could still fail.
- The `response` field of the response body **must** be set to the appropriate return type if the RPC
was not long-running: `google.protobuf.Empty` for most Delete RPCs, or the
resource itself for soft delete ([AEP-164][aep-164]).
- Both the `response_type` and `metadata_type` fields **must** be specified
(even if they are `google.protobuf.Empty`).
- The `response` field of the response body **must** be an empty object
to be consistent with the appropriate return type if the method was not long-running.
- Both the `response_type` and `metadata_type` fields **must** be specified.

{% tab proto -%}

Expand All @@ -140,9 +135,8 @@ rpc DeleteBook(DeleteBookRequest) returns (google.longrunning.Operation) {
}
```

- The response type **must** be set to the appropriate return type if the RPC
was not long-running: `google.protobuf.Empty` for most Delete RPCs, or the
resource itself for soft delete ([AEP-164][aep-164]).
- The `response` field of the response **must** be `google.protobuf.Empty`
to be consistent with the appropriate return type if the method was not long-running.
- Both the `response_type` and `metadata_type` fields **must** be specified
(even if they are `google.protobuf.Empty`).

Expand Down Expand Up @@ -175,7 +169,7 @@ message DeletePublisherRequest {
string name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference) = {
type: "library.googleapis.com/Publisher"
type: "library.example.com/Publisher"
}];

// If set to true, any books from this publisher will also be deleted.
Expand All @@ -189,94 +183,6 @@ is `false` (or unset) and child resources are present.

{% endtabs %}

### Protected delete

Sometimes, it may be necessary for users to ensure that no changes have been
made to a resource that is being deleted. If a resource provides an [etag][],
the delete request **may** accept the etag (as either required or optional):

{% tab oas -%}

{% sample 'protected_delete.oas.yaml', 'paths' %}

If the etag is provided and does not match the server-computed etag, the
request **must** fail with `412 Precondition Failed`.

{% tab proto -%}

```proto
message DeleteBookRequest {
// The name of the book to delete.
// Format: publishers/{publisher}/books/{book}
string name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference) = {
type: "library.googleapis.com/Book"
}];

// Optional. The etag of the book.
// If this is provided, it must match the server's etag.
string etag = 2;
}
```

If the etag is provided and does not match the server-computed etag, the
request **must** fail with a `ABORTED` error code.

{% endtabs %}

**Note:** Declarative-friendly resources ([AEP-128][aep-128]) **must** provide the `etag`
field for Delete requests.

### Delete if existing

If the service uses client-assigned resource names, `Delete` methods **may**
expose a `bool allow_missing` field, which will cause the method to succeed in
the event that the user attempts to delete a resource that is not present (in
which case the request is a no-op):

{% tab oas -%}

{% sample 'delete_if_existing.oas.yaml', 'paths' %}

More specifically, the `allow_missing` flag triggers the following behavior:

- If the method call is on a resource that does not exist, the request is a
no-op.
- Conditional headers (e.g. `If-Match`) are ignored.
- If the method call is on a resource that already exists, the resource is
deleted (subject to other checks).

{% tab proto -%}

```proto
message DeleteBookRequest {
// The book to delete.
// Format: publishers/{publisher}/books/{book}
string name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference).type = "library.googleapis.com/Book"
];

// If set to true, and the book is not found, the request will succeed
// but no action will be taken on the server
bool allow_missing = 2;
}
```

More specifically, the `allow_missing` flag triggers the following behavior:

- If the method call is on a resource that does not exist, the request is a
no-op.
- The `etag` field is ignored.
- If the method call is on a resource that already exists, the resource is
deleted (subject to other checks).

{% endtabs %}

**Note:** Declarative-friendly resources ([AEP-128][aep-128]) **should** expose the
`bool allow_missing` field.

### Errors

If the user does not have permission to access the resource, regardless of
Expand Down
42 changes: 0 additions & 42 deletions aep/general/0135/delete_if_existing.oas.yaml

This file was deleted.

43 changes: 0 additions & 43 deletions aep/general/0135/protected_delete.oas.yaml

This file was deleted.

0 comments on commit 56534bc

Please sign in to comment.