Skip to content

Commit

Permalink
Allow shortcut specifying InlineScript as just a string (#605)
Browse files Browse the repository at this point in the history
* Allow shortcut specifying InlineScript as just a string

Signed-off-by: Thomas Farr <[email protected]>

* Add test

Signed-off-by: Thomas Farr <[email protected]>

* Allow arbitrary script language

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Thomas Farr <[email protected]>
  • Loading branch information
Xtansia authored Oct 9, 2024
1 parent 6845fc7 commit 6b63759
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- 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))
- Added ability to pass `InlineScript` as a simple string ([#605](https://github.com/opensearch-project/opensearch-api-specification/pull/605))

### Changed

Expand Down
39 changes: 24 additions & 15 deletions spec/schemas/_common.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -451,22 +451,31 @@ components:
- title: stored
$ref: '#/components/schemas/StoredScriptId'
InlineScript:
allOf:
- $ref: '#/components/schemas/ScriptBase'
- type: object
properties:
lang:
$ref: '#/components/schemas/ScriptLanguage'
options:
type: object
additionalProperties:
type: string
source:
description: The script source.
type: string
required:
- source
oneOf:
- title: source

This comment has been minimized.

Copy link
@amberzsy

amberzsy Oct 17, 2024

Contributor

why do we need title source here?

This comment has been minimized.

Copy link
@Xtansia

Xtansia Oct 17, 2024

Author Collaborator

It's a work around to flag to the Java client that when deserializing if it encounters a direct string it can map it to the source field of the object type.

type: string
- allOf:
- $ref: '#/components/schemas/ScriptBase'
- type: object
properties:
lang:
$ref: '#/components/schemas/ScriptLanguage'
options:
type: object
additionalProperties:
type: string
source:
description: The script source.
type: string
required:
- source
ScriptLanguage:
anyOf:

This comment has been minimized.

Copy link
@amberzsy

amberzsy Oct 17, 2024

Contributor

should this be oneOf instead? i don't think ScriptLanguage can do multiple language at same request

This comment has been minimized.

Copy link
@amberzsy

amberzsy Oct 17, 2024

Contributor

also i don't see the test covered for this particular specs below.
the major change in this PR is to add arbitrary ScriptLanguage, should we add the corresponding test?

This comment has been minimized.

Copy link
@Xtansia

Xtansia Oct 17, 2024

Author Collaborator

oneOf means the value must only validate against a single one of the schemas, anyOf means it may validate against one or more.

In this case if you provide one of the BuiltinScriptLanguage values, it will also validate successfully against the "custom" string schema, meaning it would fail if it was a oneOf.

This comment has been minimized.

Copy link
@amberzsy

amberzsy Oct 19, 2024

Contributor

yeah, the major difference is

  1. anyOf is a regular OR-combination of JSON sub-schemas.
  2. oneOf is an exclusive OR-combination, or an XOR-combination, of JSON sub-schemas.

when you use anyOf for the items type, the elements can be of any type out of those and the array can contain the mixed items. Means you can have one item of type 1 and another item of type 2. Which i don't think that's the correct API definition for this case. I would prefer use oneOf which consistent with all other API definition (e.g )

This comment has been minimized.

Copy link
@Xtansia

Xtansia Oct 20, 2024

Author Collaborator

Which array are you referring to?

But again technically this can't directly be changed to a oneOf as for example "painless" would validate against both BuiltinScriptLanguage and type: string, making it invalid according to the oneOf schema.

To make it a oneOf we'd have to change it to:

ScriptLanguage:
      oneOf:
        - title: builtin
          $ref: '#/components/schemas/BuiltinScriptLanguage'
        - title: custom
          type: string
          not: 
            $ref: '#/components/schemas/BuiltinScriptLanguage'

To say that the "custom" variant can be a string but not one of the builtin ones.

- title: builtin
$ref: '#/components/schemas/BuiltinScriptLanguage'
- title: custom
type: string
BuiltinScriptLanguage:
type: string
enum:
- expression
Expand Down
78 changes: 78 additions & 0 deletions tests/default/_core/search/query/script.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
$schema: ../../../../../json_schemas/test_story.schema.yaml

description: Test ScriptQuery functionality.

prologues:
- path: /movies
method: PUT
request:
payload:
mappings:
properties:
title:
type: keyword
year:
type: integer

- path: /movies/_bulk
method: POST
parameters:
refresh: true
request:
content_type: application/x-ndjson
payload:
- index:
_id: 1
- title: The Lion King
year: 1994
- index:
_id: 2
- title: Beauty and the Beast
year: 1991
- index:
_id: 3
- title: Aladdin
year: 1992
- index:
_id: 4
- title: The Little Mermaid
year: 1989
status: [200]

epilogues:
- path: /movies
method: DELETE
status: [200, 404]

chapters:
- synopsis: Search using ScriptQuery with filtering to movies with odd years.
path: /{index}/_search
parameters:
index: movies
method: POST
request:
payload:
query:
bool:
filter:
script:
script: "doc['year'].value % 2 == 1"
sort:
- year:
order: desc
response:
status: 200
payload:
timed_out: false
hits:
total:
value: 2
hits:
- _id: '2'
_source:
title: Beauty and the Beast
year: 1991
- _id: '4'
_source:
title: The Little Mermaid
year: 1989

0 comments on commit 6b63759

Please sign in to comment.