Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 404 spec for delete index, get document, delete document APIs #589

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `/_plugins/_ml/agents/_register`, `/_plugins/_ml/connectors/_create`, `DELETE /_plugins/_ml/agents/{agent_id}`, `DELETE /_plugins/_ml/connectors/{connector_id}` ([#228](https://github.com/opensearch-project/opensearch-api-specification/issues/228))
- Added the `context` query param to the `put_script` APIs ([#586](https://github.com/opensearch-project/opensearch-api-specification/pull/586))
- Added `persian_stem` filter ([#592](https://github.com/opensearch-project/opensearch-api-specification/pull/592))
- Added `404` response for `DELETE /{index}`, `GET /{index}/_doc/{id}`, `DELETE /{index}/_doc/{id}` ([#589](https://github.com/opensearch-project/opensearch-api-specification/pull/589))

### Changed

Expand Down
10 changes: 8 additions & 2 deletions spec/namespaces/_core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,8 @@ paths:
responses:
'200':
$ref: '#/components/responses/get@200'
'404':
$ref: '#/components/responses/get@404'
head:
operationId: exists.0
x-operation-group: exists
Expand Down Expand Up @@ -1468,6 +1470,8 @@ paths:
responses:
'200':
$ref: '#/components/responses/delete@200'
'404':
$ref: '#/components/responses/delete@404'
/{index}/_explain/{id}:
get:
operationId: explain.0
Expand Down Expand Up @@ -2837,11 +2841,12 @@ components:
creation_time:
type: integer
format: int64
delete@200:
delete@200: &WriteResponseBase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting syntax! Where can I read about & and *?

content:
application/json:
schema:
$ref: '../schemas/_common.yaml#/components/schemas/WriteResponseBase'
delete@404: *WriteResponseBase
delete_all_pits@200:
content:
application/json:
Expand Down Expand Up @@ -2925,11 +2930,12 @@ components:
required:
- fields
- indices
get@200:
get@200: &getResult
content:
application/json:
schema:
$ref: '../schemas/_core.get.yaml#/components/schemas/GetResult'
get@404: *getResult
get_all_pits@200:
content:
application/json:
Expand Down
7 changes: 7 additions & 0 deletions spec/namespaces/indices.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,8 @@ paths:
responses:
'200':
$ref: '#/components/responses/indices.delete@200'
'404':
$ref: '#/components/responses/indices.delete@404'
/{index}/_alias:
get:
operationId: indices.get_alias.2
Expand Down Expand Up @@ -2400,6 +2402,11 @@ components:
application/json:
schema:
$ref: '../schemas/_common.yaml#/components/schemas/IndicesResponseBase'
indices.delete@404:
content:
application/json:
schema:
$ref: '../schemas/indices._common.yaml#/components/schemas/IndexErrorCause'
indices.delete_alias@200:
content:
application/json:
Expand Down
4 changes: 4 additions & 0 deletions spec/schemas/_common.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2241,3 +2241,7 @@ components:
BatchSize:
type: integer
format: int64
ResourceType:
type: string
enum:
- index_or_alias
26 changes: 26 additions & 0 deletions spec/schemas/indices._common.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1010,3 +1010,29 @@ components:
- mappings
- order
- settings
IndexErrorCause:
type: object
properties:
type:
description: The type of error
type: string
reason:
description: A human-readable explanation of the error, in english
type: string
root_cause:
type: array
items:
$ref: '#/components/schemas/IndexErrorCause'
index:
$ref: '_common.yaml#/components/schemas/IndexName'
index_uuid:
$ref: '_common.yaml#/components/schemas/Uuid'
resource.id:
$ref: '_common.yaml#/components/schemas/IndexName'
resource.type:
$ref: '_common.yaml#/components/schemas/ResourceType'
required:
- type
additionalProperties:
title: metadata
description: Additional details about the error.
16 changes: 16 additions & 0 deletions tests/default/indices/doc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,19 @@ chapters:
id: '1'
response:
status: 200
- synopsis: Retrieve a non existing document.
path: /{index}/_doc/{id}
method: GET
parameters:
index: movies
id: '1'
response:
status: 404
- synopsis: Delete a non existing document.
path: /{index}/_doc/{id}
method: DELETE
parameters:
index: movies
id: '1'
response:
status: 404
10 changes: 9 additions & 1 deletion tests/default/indices/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,12 @@ chapters:
version: '>= 2.0'
parameters:
index: books,games
cluster_manager_timeout: 10s
cluster_manager_timeout: 10s

- synopsis: Delete non existing index `movies`.
path: /{index}
method: DELETE
parameters:
index: movies
response:
status: 404
2 changes: 1 addition & 1 deletion tools/src/tester/ChapterReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default class ChapterReader {
response.status = e.response.status
response.content_type = e.response.headers['content-type']?.split(';')[0]
const payload = this.#deserialize_payload(e.response.data, response.content_type)
if (payload !== undefined) response.payload = payload.error
if (payload !== undefined) response.payload = payload.error ?? payload
response.message = payload.error?.reason ?? e.response.statusText
this.logger.info(`<= ${response.status} (${response.content_type}) | ${response.payload !== undefined ? to_json(response.payload) : response.message}`)
}
Expand Down
29 changes: 29 additions & 0 deletions tools/tests/tester/ChapterReader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,35 @@ describe('ChapterReader', () => {
}]
])
})

it('sets payload to entire response when payload.error is missing', async () => {
const mock_payload = { '_data': '1', 'result': 'updated' };
const mock_error = {
response: {
status: 404,
headers: {
'content-type': 'application/json'
},
data: JSON.stringify(mock_payload),
statusText: 'Not Found'
}
};

mocked_axios.request.mockRejectedValue(mock_error);

const result = await reader.read({
id: 'id',
path: 'path',
method: 'POST'
}, new StoryOutputs());

expect(result).toStrictEqual({
status: 404,
content_type: 'application/json',
payload: mock_payload,
message: 'Not Found'
});
});
})

describe('deserialize_payload', () => {
Expand Down
Loading