feat: openapi sample with search, index, and batch API #4

millotp · 2021-11-05T09:06:44Z

This is a first sample with various API to test the capabilities of the generation.
The files are split by schema and path, to bundle all the file into one big file (dist/openapi.yml, it can be useful for swagger editor), use the command yarn build:spec.

Some types are not correct in the SearchParams because they require union (like string | boolean).

shortcuts

Great work!

We know we will break a lot of things with this new client, but we should limit the footprint if possible like keeping the naming consistent (unless the previous naming does not make sense of course)

shortcuts · 2021-11-05T09:13:07Z

spec_sample.yaml

+    post:
+      tags:
+        - search
+      operationId: searchSingle


Suggested change

operationId: searchSingle

operationId: search

We should keep the same naming

I agree but the operationId must be unique in the file, how do you differentiate all of those:

POST /1/indexes/{indexName}/query

GET /1/indexes/{indexName}

POST /1/indexes/*/queries

IIRC there's two search method but they are not both available on the client:

on an initialized index: /1/indexes/{indexName}/query

on the client: /1/indexes/*/queries

Since we don't have any init method yet we should only focus on the second

shortcuts · 2021-11-05T09:47:07Z

spec_sample.yaml

+        - search
+      operationId: searchSingle
+      summary: Get search results
+      parameters:


We are missing the query here

Just to be sure, we want to get rid of the other params (appId, apiKey and IndexName) to replicate what's in the current javascript client right ? As we're going to implement the init method.

Indeed

I guess we could have two spec files, one without init (this one), and a new one once we know how to generate the init?

Ok let's do this 🚀

spec_sample.yaml

bodinsamuel

Looking good 🙏🏻

spec_sample.yaml

damcou · 2021-11-05T10:09:01Z

I'm being tatillon, but shouldn't we stay consistent regarding YAML file extension between .yml and .yaml ? :p

bodinsamuel · 2021-11-05T10:11:44Z

Jean Michel Tatillon

spec_sample.yaml

damcou

Great work !

Just left comments on some structural aspect of the POC as you saw ;) .

shortcuts

Awesome :D

I'm not able to generate from the bundle yet, maybe some missing compatibilities between openapi/swagger CLI?

shortcuts · 2021-11-09T09:57:55Z

package.json

@@ -6,6 +6,7 @@
  ],
  "scripts": {
    "generate": "PACKAGE_VERSION='4.11.0' TS_POST_PROCESS_FILE='yarn prettier --write .' yarn openapi-generator-cli generate --generator-key client && yarn install",
+    "build:spec": "yarn swagger-cli bundle openapi_spec/spec.yml --outfile dist/openapi.yml --type yaml",


Have you been able to generate a client after building? I got some errors using the outputted file

Not with the generated file, the option --skip-validate-spec isn't taken into account for some reason, but it works with the original openapi_spec/spec.yml file.

shortcuts · 2021-11-09T09:59:36Z

openapi_spec/schemas/SearchParams.yml

+    default: ""
+  similarQuery:
+    type: string
+    description: Overrides the query parameter and performs a more generic search that can be used to find “similar” results.


Suggested change

description: Overrides the query parameter and performs a more generic search that can be used to find “similar” results.

description: Overrides the query parameter and performs a more generic search that can be used to find "similar" results.

Does it work without weird quotes?

That's a copy paste from the doc, I didn't notice them, thanks

shortcuts · 2021-11-09T10:01:09Z

openapi_spec/schemas/SearchParams.yml

+    type: string
+    description: Filter the query with numeric, facet and/or tag filters.
+    default: ""
+    # There could be a pattern for this one (complicated one)


I think it's close enough for our testing phase, but let's not forget later

shortcuts · 2021-11-09T10:06:10Z

openapi_spec/paths/indexes/batch.yml

@@ -0,0 +1,58 @@
+post:
+  tags:
+    - object


This is bundled with the search package atm, do we want to change it?

shortcuts · 2021-11-09T10:09:53Z

openapi_spec/paths/indexes/searchMulti.yml

+post:
+  tags:
+    - search
+  operationId: searchMulti


(Not important at all for now but searchMulti is a bit vague)

shortcuts · 2021-11-09T10:15:30Z

openapi_spec/paths/indexes/searchMulti.yml

+            strategy:
+              type: string
+              enum: ["none", "stopIfEnoughMatches"]


I think this should be an object with only the strategy option for now, it's closer to what we have actually and avoid breaking the API if we add requestOptions in the future

You mean removing the requests property ?

I mean search methods both receive 2 parameters, query (or queries here) and requestOptions (which contains the strategy), we should have the same shape here.

From the doc I understand that the body should contain the requests and maybe the strategy, it's not in the requestOptions I think.

Is it a usual in the API clients to bundle the real property in a requests property ?

From the doc I understand that the body should contain the requests and maybe the strategy, it's not in the requestOptions I think.

Oh I see, I was referring to our API client structure and not the REST API. Since we pass options to our transporter etc. it may differ

shortcuts · 2021-11-09T10:15:53Z

openapi_spec/paths/indexes/search.yml

+    - $ref: "../../parameters.yml#/AppId"
+    - $ref: "../../parameters.yml#/ApiKey"
+    - $ref: "../../parameters.yml#/IndexName"
+  requestBody:


I think we are missing the query here

It's in the search params with the rest

bodinsamuel

LGTM 🪨

Potential next step is to finish one endpoint entirely so we can highlight everything, exhaustive listing of errors, all parameters, all responses, etc...

openapi_spec/spec.yml

bodinsamuel · 2021-11-10T09:59:47Z

openapi_spec/responses/BadRequest.yml

+content:
+  application/json:
+    schema:
+      $ref: '../schemas/Error.yml'


Might be confusing to reuse Error at this point since the example won't reflect the actual error

I don't know if we can be exhaustive about the actual error, I agree that this is bad for the doc but the error can be anything, even for a single endpoint.
What do you have in mind to reflect the actual error ?

we probably need to dig a bit deeper in the API, but not for this PR indeed

bodinsamuel · 2021-11-10T09:59:52Z

openapi_spec/responses/IndexNotFound.yml

+content:
+  application/json:
+    schema:
+      $ref: '../schemas/Error.yml'


openapi_spec/paths/indexes/search.yml

openapi_spec/paths/indexes/saveObject.yml

bodinsamuel · 2021-11-10T10:01:56Z

openapi_spec/paths/indexes/multipleQueries.yml

+                type: object
+                additionalProperties: false
+                properties:
+                  indexName:


openapi_spec/paths/indexes/multipleQueries.yml

bodinsamuel · 2021-11-10T10:05:17Z

openapi_spec/schemas/SearchParams.yml

@@ -0,0 +1,451 @@
+type: object


This file won't scale well, I suppose we need to split it or add comments/line break/grouping because it's already hardly readable.

I will split it in another PR, I'm still thinking what's best

feat: openapi sample with search, index, and batch API

6354944

millotp requested review from damcou and shortcuts November 5, 2021 09:06

millotp self-assigned this Nov 5, 2021

shortcuts reviewed Nov 5, 2021

View reviewed changes

bodinsamuel reviewed Nov 5, 2021

View reviewed changes

rename to yml

da5f55a

damcou reviewed Nov 5, 2021

View reviewed changes

spec_sample.yaml Outdated Show resolved Hide resolved

damcou reviewed Nov 5, 2021

View reviewed changes

spec_sample.yaml Outdated Show resolved Hide resolved

damcou reviewed Nov 5, 2021

View reviewed changes

millotp added 3 commits November 5, 2021 16:02

hopefully I've answered everything

de4a124

some fixups

7cf934a

more fixups

47fc15d

millotp requested review from damcou, shortcuts and bodinsamuel November 9, 2021 09:55

shortcuts reviewed Nov 9, 2021

View reviewed changes

millotp added 2 commits November 9, 2021 14:47

use simple quote and change tags

cb2e499

real error

f2a4845

bodinsamuel approved these changes Nov 10, 2021

View reviewed changes

create common response file with taskID and objectID

4966251

shortcuts approved these changes Nov 10, 2021

View reviewed changes

millotp merged commit 277532e into main Nov 10, 2021

millotp deleted the feat/spec-sample branch November 10, 2021 13:50

shortcuts pushed a commit that referenced this pull request Apr 22, 2022

Merge pull request #4 from algolia/feat/spec-sample

0139764

This was referenced May 24, 2022

chore: prepare release 2022-05-24 #541

Closed

chore: prepare release 2022-05-24 #542

Closed

chore: prepare release 2022-05-25 #545

Closed

This was referenced May 25, 2022

chore: prepare release 2022-05-25 #546

Closed

chore: prepare release 2022-05-25 #547

Closed

chore: prepare release 2022-05-25 #548

Closed

chore: prepare release 2022-05-25 #549

Closed

	description: Overrides the query parameter and performs a more generic search that can be used to find “similar” results.
	description: Overrides the query parameter and performs a more generic search that can be used to find "similar" results.

@@ @@ -0,0 +1,58 @@ @@
+              post:
+                tags:
+                  - object

feat: openapi sample with search, index, and batch API #4

feat: openapi sample with search, index, and batch API #4

Conversation

millotp commented Nov 5, 2021 • edited Loading

shortcuts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bodinsamuel left a comment

Choose a reason for hiding this comment

damcou commented Nov 5, 2021

bodinsamuel commented Nov 5, 2021

damcou left a comment

Choose a reason for hiding this comment

shortcuts left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

millotp Nov 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shortcuts Nov 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bodinsamuel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

millotp commented Nov 5, 2021 •

edited

Loading

shortcuts left a comment •

edited

Loading

millotp Nov 9, 2021 •

edited

Loading

shortcuts Nov 9, 2021 •

edited

Loading