Add API Spec for Geospatial plugin #893

weijia-aws · 2025-05-12T23:01:16Z

Description

This PR adds API Spec for geospatial plugin

Tests

npm run test:spec -- --opensearch-insecure  --tests tests/default/geospatial 

> [email protected] test:spec
> ts-node tools/src/tester/test.ts --opensearch-insecure --tests tests/default/geospatial

OpenSearch 2.19.0

PASSED  geojson/upload.yaml (tests/default/geospatial/geojson/upload.yaml)
PASSED  ip2geo/datasource.yaml (tests/default/geospatial/ip2geo/datasource.yaml)
PASSED  upload/stats.yaml (tests/default/geospatial/upload/stats.yaml)
PASSED  ip2geo/datasource/settings.yaml (tests/default/geospatial/ip2geo/datasource/settings.yaml)

Tested 8/611 paths.

npm run lint:spec

> [email protected] lint:spec
> ts-node tools/src/linter/lint.ts

Validating /Users/zweijia/IdeaProjects/opensearch-api-specification/spec ...
No errors found.

Issues Resolved

Closes #238
Closes opensearch-project/geospatial#658

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2025-05-13T11:41:46Z

Changes Analysis

Commit SHA: 858cb84
Comparing To SHA: bddc88a

API Changes

Summary

├─┬Paths
│ ├──[➕] path (9105:3)
│ ├──[➕] path (9049:3)
│ ├──[➕] path (9122:3)
│ ├──[➕] path (9064:3)
│ └──[➕] path (9184:3)
└─┬Components
  ├──[➕] requestBodies (28262:7)
  ├──[➕] requestBodies (28274:7)
  ├──[➕] requestBodies (28268:7)
  ├──[➕] requestBodies (28256:7)
  ├──[➕] responses (31500:7)
  ├──[➕] responses (31518:7)
  ├──[➕] responses (31524:7)
  ├──[➕] responses (31506:7)
  ├──[➕] responses (31560:7)
  ├──[➕] responses (31548:7)
  ├──[➕] responses (31512:7)
  ├──[➕] responses (31536:7)
  ├──[➕] responses (31554:7)
  ├──[➕] responses (31530:7)
  ├──[➕] responses (31542:7)
  ├──[➕] parameters (20243:7)
  ├──[➕] parameters (20261:7)
  ├──[➕] parameters (20255:7)
  ├──[➕] parameters (20249:7)
  ├──[➕] schemas (53477:7)
  ├──[➕] schemas (53625:7)
  ├──[➕] schemas (53712:7)
  ├──[➕] schemas (53642:7)
  ├──[➕] schemas (53590:7)
  ├──[➕] schemas (53418:7)
  ├──[➕] schemas (53705:7)
  ├──[➕] schemas (53453:7)
  ├──[➕] schemas (53741:7)
  ├──[➕] schemas (53613:7)
  ├──[➕] schemas (53659:7)
  ├──[➕] schemas (53630:7)
  ├──[➕] schemas (53522:7)
  ├──[➕] schemas (53462:7)
  ├──[➕] schemas (53596:7)
  ├──[➕] schemas (53569:7)
  ├──[➕] schemas (53694:7)
  ├──[➕] schemas (53683:7)
  ├──[➕] schemas (53506:7)
  ├──[➕] schemas (53647:7)
  ├──[➕] schemas (53403:7)
  ├──[➕] schemas (53534:7)
  ├──[➕] schemas (53671:7)
  ├──[➕] schemas (53557:7)
  ├──[➕] schemas (53502:7)
  ├──[➕] schemas (53578:7)
  └──[➕] schemas (53608:7)

Document Element	Total Changes	Breaking Changes
paths	5	0
components	46	0

Total Changes: 51
Additions: 51

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/15056800388/artifacts/3135519884

API Coverage

	Before	After	Δ
Covered (%)	663 (64.94 %)	663 (64.94 %)	0 (0 %)
Uncovered (%)	358 (35.06 %)	358 (35.06 %)	0 (0 %)
Unknown	53	61	8

github-actions · 2025-05-13T11:51:45Z

Spec Test Coverage Analysis

Total	Tested
611	609 (99.67 %)

CHANGELOG.md

spec/namespaces/geospatial.yaml

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia · 2025-05-14T00:13:13Z

spec/namespaces/geospatial.yaml

+    post:
+      operationId: geospatial.geojson_upload_post.0
+      x-operation-group: geospatial.geojson_upload_post
+      x-version-added: '2.11'
+      description: |- 
+        Use an OpenSearch query to upload `GeoJSON`, operation will fail if index exists.
+        - When type is `geo_point`, only Point geometry is allowed
+        - When type is `geo_shape`, all geometry types are allowed (Point, MultiPoint, LineString, MultiLineString, Polygon, MultiPolygon, GeometryCollection, Envelope).
+      requestBody:
+        $ref: '#/components/requestBodies/geospatial.geojson_upload_post'
+      responses:
+        '200':
+          $ref: '#/components/responses/geospatial.geojson_upload_post@200'
+        '400':
+          $ref: '#/components/responses/geospatial.geojson_upload_post@400'
+    put:
+      operationId: geospatial.geojson_upload_put.0
+      x-operation-group: geospatial.geojson_upload_put
+      x-version-added: '2.11'


As these appear to be functionally identical, I'd give these the same operation group to allow generated clients to collapse them into one operation. Also so that the grouping reads more like the action being performed I'd potentially rearrange the naming to geospatial.upload_geojson.

I tried to do that initially, but it violates lint:spec. Here's what it complains:

Validating /Users/zweijia/IdeaProjects/opensearch-api-specification/spec ... [ERROR] Errors found: { file: 'namespaces/geospatial.yaml', location: 'Operation Group: geospatial.geojson_upload', message: "2 'geospatial.geojson_upload' operations must have identical description property." } { file: 'namespaces/geospatial.yaml', location: 'Operation Group: geospatial.geojson_upload', message: "2 'geospatial.geojson_upload' operations must have an identical set of responses." } [ERROR]

Ah, I just noticed the operation will fail if index exists. vs regardless if index exists., that does mean they shouldn't actually be the same operation. Naming will be tricky to communicate that difference.

spec/namespaces/geospatial.yaml

Xtansia · 2025-05-14T00:19:14Z

spec/namespaces/geospatial.yaml

+  /_plugins/geospatial/ip2geo/datasource:
+    get:
+      operationId: geospatial.ip_to_geo_get_all_datasource.0
+      x-operation-group: geospatial.ip_to_geo_get_all_datasource
+      x-version-added: '2.11'
+      description: Get information about all IP2Geo data sources.
+      externalDocs:
+        url: 'https://docs.opensearch.org/docs/latest/ingest-pipelines/processors/ip2geo/'
+      responses:
+        '200':
+          $ref: '#/components/responses/geospatial.ip_to_geo_get_all_datasource@200'
+
+  /_plugins/geospatial/ip2geo/datasource/{datasource_name}:
+    get:
+      operationId: geospatial.ip_to_geo_get_datasource.0
+      x-operation-group: geospatial.ip_to_geo_get_datasource
+      x-version-added: '2.11'
+      description: Get information for a specific IP2Geo data source.
+      externalDocs:
+        url: 'https://docs.opensearch.org/docs/latest/ingest-pipelines/processors/ip2geo/#sending-a-get-request'
+      parameters:
+        - $ref: '#/components/parameters/geospatial.ip_to_geo_get_datasource::path.datasource_name'
+      responses:
+        '200':
+          $ref: '#/components/responses/geospatial.ip_to_geo_get_datasource@200'


As these have identical response structures, the pattern used elsewhere in the spec is to give these the same operation group, and allow generated clients to make overloads and pick the path based on whether a datasource name was provided.

Again also preferring the action as the start of the name, in this case I'd prefer geospatial.get_ip_to_geo_datasource.

I'd also be fine with these using get_ip2geo_datasource for conciseness and matching the naming of the data format, however the regex here currently disallows digits, I think it'd be reasonable to allow digits as long as it isn't the first character of either part of the group. Thoughts @dblock @nhtruong ?

As these have identical response structures, the pattern used elsewhere in the spec is to give these the same operation group, and allow generated clients to make overloads and pick the path based on whether a datasource name was provided.

Although these have the same response structure, the request is different. For the first one, it doesn't require to pass in a {datasource_name} parameter , while the second one requires. Also the description is different as well. If we move to a same group, it will violate the lint rule again.

Again also preferring the action as the start of the name, in this case I'd prefer geospatial.get_ip_to_geo_datasource.

Will do

I'd also be fine with these using get_ip2geo_datasource for conciseness and matching the naming of the data format, however the regex here currently disallows digits, I think it'd be reasonable to allow digits as long as it isn't the first character of either part of the group. Thoughts @dblock @nhtruong ?

Yeah, I was using ip2geo_datasource first, and it violate the regex

As these have identical response structures, the pattern used elsewhere in the spec is to give these the same operation group, and allow generated clients to make overloads and pick the path based on whether a datasource name was provided.

Although these have the same response structure, the request is different. For the first one, it doesn't require to pass in a {datasource_name} parameter , while the second one requires. Also the description is different as well. If we move to a same group, it will violate the lint rule again.

Based on the code https://github.com/opensearch-project/geospatial/blob/main/src/main/java/org/opensearch/geospatial/ip2geo/action/RestGetDatasourceHandler.java they're not really separate actions, as the {datasource_name} can also be a comma delimited array, so it's just "Get one or more IP2Geo data sources, defaulting to returning all if no names specified".

I see, then I can merge these two actions into one

spec/namespaces/geospatial.yaml

spec/schemas/geospatial._common.yaml

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia · 2025-05-14T22:57:28Z

spec/namespaces/geospatial.yaml

+      in: path
+      required: true
+      schema:
+        type: string


Given this can be a comma delimited array, this should be:

Suggested change

type: string

$ref: '../schemas/_common.yaml#/components/schemas/StringOrStringArray'

Xtansia

Additionally all the {datasource_name} path params can just be {name}

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia

I would update the operation group regex to:

const GROUP_REGEX = /^([a-z]+[a-z_]*[a-z]+\.)?([a-z]+[a-z0-9_]*[a-z0-9]+)$/

And rename all the ip_to_geos to ip2geo

Xtansia · 2025-05-15T00:16:19Z

spec/namespaces/geospatial.yaml

+        '200':
+          $ref: '#/components/responses/geospatial.get_upload_stats@200'
+
+  /_plugins/geospatial/ip2geo/datasource/{name}:


You still need the non {name} path variant, just giving them the same operation-group, description and response $ref

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia · 2025-05-15T22:33:23Z

tools/src/linter/components/Operation.ts

@@ -11,7 +11,7 @@ import { type OperationSpec, type ValidationError } from 'types'
 import _ from 'lodash'
 import ValidatorBase from './base/ValidatorBase'

-const GROUP_REGEX = /^([a-z]+[a-z_]*[a-z]+\.)?([a-z]+[a-z_]*[a-z]+)$/
+const GROUP_REGEX = /^([a-z]+[a-z_]*[a-z]+\.)?([a-z]+[a-z0-9_]*[a-z0-9]+)$/


Looks like this regex appears in error messages which are tested verbatim, so will need updating:

https://github.com/opensearch-project/opensearch-api-specification/blob/main/tools/tests/linter/Operation.test.ts#L22

https://github.com/opensearch-project/opensearch-api-specification/blob/main/tools/tests/linter/Operation.test.ts#L26

Signed-off-by: Weijia Zhao <[email protected]>

weijia-aws requested review from dblock, harshavamsi, sachetalva, nhtruong, Xtansia, VachaShah, Tokesh and aabeshov as code owners May 12, 2025 23:01

weijia-aws mentioned this pull request May 12, 2025

Onboard OpenSearch plugins to Smoke tests framework opensearch-project/geospatial#724

Closed

weijia-aws marked this pull request as draft May 13, 2025 21:31

weijia-aws force-pushed the geospatial branch from 421aa8f to e1608f5 Compare May 13, 2025 22:15

weijia-aws marked this pull request as ready for review May 13, 2025 22:17

weijia-aws marked this pull request as draft May 13, 2025 23:15

weijia-aws force-pushed the geospatial branch from e1608f5 to 2c248c6 Compare May 13, 2025 23:15

weijia-aws marked this pull request as ready for review May 13, 2025 23:16

Xtansia requested changes May 13, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

spec/namespaces/geospatial.yaml Outdated Show resolved Hide resolved

Add API Spec for Geospatial plugin

2017fb3

Signed-off-by: Weijia Zhao <[email protected]>

weijia-aws force-pushed the geospatial branch from 2c248c6 to 2017fb3 Compare May 13, 2025 23:34

Xtansia requested changes May 14, 2025

View reviewed changes

Address comments

f10efc2

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia reviewed May 14, 2025

View reviewed changes

Address comments

1ffcc48

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia reviewed May 15, 2025

View reviewed changes

Update GROUP_REGEX to allow number values

0f2e935

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia reviewed May 15, 2025

View reviewed changes

Update unit test

858cb84

Signed-off-by: Weijia Zhao <[email protected]>

Xtansia approved these changes May 16, 2025

View reviewed changes

Xtansia merged commit 336d60f into opensearch-project:main May 16, 2025
34 checks passed

	type: string
	$ref: '../schemas/_common.yaml#/components/schemas/StringOrStringArray'

Add API Spec for Geospatial plugin #893

Add API Spec for Geospatial plugin #893

Uh oh!

Conversation

weijia-aws commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Issues Resolved

Uh oh!

github-actions bot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes Analysis

API Changes

Summary

Report

API Coverage

Uh oh!

github-actions bot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Spec Test Coverage Analysis

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xtansia left a comment

Choose a reason for hiding this comment

Uh oh!

Xtansia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

weijia-aws commented May 12, 2025 •

edited

Loading

github-actions bot commented May 13, 2025 •

edited

Loading

github-actions bot commented May 13, 2025 •

edited

Loading