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

Magento REST API Schema (Swagger) is not compatible with Search Criteria #11477

Open
careys7 opened this issue Oct 15, 2017 · 22 comments
Open

Magento REST API Schema (Swagger) is not compatible with Search Criteria #11477

careys7 opened this issue Oct 15, 2017 · 22 comments
Assignees
Labels
Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog feature request Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: dev in progress Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@careys7
Copy link
Member

careys7 commented Oct 15, 2017

Re-opening #7511 per comment #7511 (comment) as this affects 2.0 - 2.2 and hasn't been resolved.

Preconditions

  1. Magento 2 CE or EE (all versions)

Steps to reproduce

  1. View Magento-generated Swagger Specification

Expected result

  1. Search criteria described in format compatible with Open API / Swagger specification.

Actual result

  1. Format not described in a Swagger-compatible manner.
  2. Making searchCriteria-related requests using swagger-api/swagger-codegen clients produces an HTTP 400 response from Magento when request sent in described format.

The Magento dev docs correctly describe REST API search criteria field groups and filters usage as needing an index per-field group / filter:

searchCriteria[filter_groups][<index>][filters][<index>][field=<field_name>]
searchCriteria[filter_groups][<index>][filters][<index>][value=<search_value>]
searchCriteria[filter_groups][<index>][filters][<index>][condition_type=<operator>]

When viewing the generated Swagger documentation, search criteria is described as follows (without a numerical index):

searchCriteria[filterGroups][][filters][][field]

Sending a request using searchCriteria in the format described by schema.json (without the numerical index) produces an HTTP 400 response. It also does not allow a user to define more than one filterGroup or filter, and breaks swagger-codegen clients.

The searchCriteria implementation doesn't appear to be compatible with the Open API Specification in the current format.


Possible solutions

  1. Provide Swagger code-gen templates to workaround the spec
  2. Implement a searchCriteria builder endpoint to accept a searchCritera POST body and reply with the query string.
  3. Change searchCriteria implementation
@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Oct 15, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Oct 20, 2017
@magento-engcom-team magento-engcom-team self-assigned this Oct 20, 2017
@dmanners dmanners added the Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog label Oct 25, 2017
@dmanners
Copy link
Contributor

Thanks for this @careys7 when processing a PR this same issue was raised by the QA team. Hopefully now it will be reproducible. If not please ping me and I can show the steps I found it with.

@careys7
Copy link
Member Author

careys7 commented Oct 27, 2017

Thanks @dmanners, reading your comments on #11421 it sounds like you have had a trouble running a query with searchCritera?

The searchCriteria itself does work well, but it has to be constructed in a way that isn't described properly by the Swagger specification.

I think the best fix for this would be to describe searchCriteria like this in the resources that use it:

{
  "/V1/products": {
    "get": {
      "tags": [
        "catalogProductRepositoryV1"
      ],
      "description": "Get product list",
      "operationId": "catalogProductRepositoryV1GetListGet",
      "parameters": [
        {
          "name": "searchCriteria",
          "in": "query",
          "type": "string",
          "description": "Search criteria query string generated by /V1/searchCriteria/generate"
        }
      ]
    }
  }
}

And then add a search critieria generator like this:

{
  "swagger": "2.0",
  "info": {
    "version": "2.1",
    "title": "Magento Enterprise"
  },
  "host": "magento.com",
  "basePath": "/rest/default",
  "schemes": [
    "http"
  ],
  "tags": [
    {
      "name": "frameworkApiSearchCriteria",
      "description": "Search Criteria Management / Generation"
    }
  ],
  "paths": {
    "/V1/searchCriteria/generate": {
      "post": {
        "tags": [
          "frameworkApiSearchCriteria"
        ],
        "description": "Convert a Magento\\Framework\\Api\\SearchCriteriaInterface to an HTTP query string that can be used in subsequent requests to search for content",
        "operationId": "frameworkApiSearchCriteriaGeneratePost",
        "parameters": [
          {
            "name": "$body",
            "in": "body",
            "schema": {
              "required": [
                "searchCriteria"
              ],
              "properties": {
                "searchCriteria": {
                  "$ref": "#/definitions/framework-api-search-criteria-interface"
                }
              },
              "type": "object"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "200 Success.",
            "schema": {
              "type": "string"
            }
          },
          "400": {
            "description": "400 Bad Request",
            "schema": {
              "$ref": "#/definitions/error-response"
            }
          },
          "401": {
            "description": "401 Unauthorized",
            "schema": {
              "$ref": "#/definitions/error-response"
            }
          },
          "default": {
            "description": "Unexpected error",
            "schema": {
              "$ref": "#/definitions/error-response"
            }
          }
        }
      }
    }
  }
}

This way, an API client can perform the following:

Construct search criteria, JSON encode using Swagger client, and submit to Magento:

POST http://example.org/rest/V1/searchCriteria/generate
{
    "searchCriteria": {"pageSize":10, "currentPage":2}
}

HTTP 200 Ok
searchCriteria[pageSize]=10&searchCriteria[currentPage]=2

Client receives the converted criteria, and can supply it with their query:

GET http://example.org/rest/V1/products/?searchCriteria[pageSize]=10&searchCriteria[currentPage]=2

This solves the problem where Swagger clients can't currently specify search criteria at all, but we are still only able to define one query parameter "searchCriteria", when there should be more unique parameters, like:

  • searchCriteria[pageSize]
  • searchCriteria[currentPage]

etc.

Potentially the web API could accept an searchCriteriaEncoded, which would allow the generation endpoint to encode this as a single value, which can be assigned to a single query parameter in the Swagger specification?

Taking the above example, this would look like this:

Construct search criteria, JSON encode using Swagger client, and submit to Magento:

POST http://example.org/rest/V1/searchCriteria/generate
{
    "searchCriteria": {"pageSize":10, "currentPage":2}
}

HTTP 200 Ok
searchCriteria%5BpageSize%5D%3D10%26searchCriteria%5BcurrentPage%5D%3D2%0D%0A

Client receives the converted criteria, and can supply it with their query:

GET http://example.org/rest/V1/products/?searchCriteriaEncoded=searchCriteria%5BpageSize%5D%3D10%26searchCriteria%5BcurrentPage%5D%3D2%0D%0A

This would be backwards-compatible with clients that have successfully implemented search criteria encoding already, who can specify the searchCriteria URL parameters as they do currently.

Going forward users would also be able to construct searchCriteria using the models built by Swagger, and POST this to Magento to have the criteria generated and encoded for them (in a Swagger Open API compliant manner).

Most importantly, the swagger code-gen clients generated are likely to compile and run :) What are your thoughts?

@dmanners
Copy link
Contributor

@careys7 thank you for the update. Yeah as I found out making the call as described in the devdocs and there is no problem but when swagger is used to build the queries then we have the problems.

Would your suggestion lead to another step in swagger for the end users? i.e. use the "Try it out" button on the searchCriteria generator and then attache that to their query or would this fix the individual "Try it out" sections for each API call?

I would love the swagger users to have the same flow as now that is:

  1. Pick API,
  2. Fill in input fields for search criteria,
  3. Select "Try it out" button,

But then actually be able to see results.

@TomashKhamlai
Copy link
Contributor

TomashKhamlai commented Oct 27, 2017

@magento-engcom-team please, recheck the status of those tickets MAGETWO-81910, MAGETWO-81904, MAGETWO-82487.

Try to use this test MAGETWO-82903.

Pay attention to some bugs that are already created and check whether they are relevant to this issue:
MAGETWO-82913, MAGETWO-82727, MAGETWO-82709, MAGETWO-73867, MAGETWO-59723.

Wrote this message because of #7511.

I have no information about 'HTTP 400 response'. Got only '500' or '200' using Search Criteria

@careys7
Copy link
Member Author

careys7 commented Oct 27, 2017

@dmanners it would result in a second step, but because search criteria can describe an endless number of filters, filter groups and condition types, I think a new endpoint to accept the criteria as JSON (which can be properly described as a Swagger model) is the most compliant to the specification.


Edit - looking at your use case (Swagger UI), we could always ensure that a sensible error message is returned to the client where a searchCriteria hasn't been defined, so that it is still really clear for users who want to experiment using the UI, eg:

GET http://magento.com/rest/v1/products

{"message":"Search criteria parameter(s) could not be found, expecting one of \"searchCriteria\" or \"searchCriteriaEncoded\". Generate this parameter using frameworkApiSearchCriteriaGeneratePost, or see devdocs for information on generating one with your API client."}

I know that the rest of the schema doesn't have meaningful examples attached (yet), but possibly the framework-api-search-criteria-interface would be a good place to start so that it appears with a filter group, filter and condition ready for the user to submit into Swagger UI?

In case it's not clear, this issue is mainly focused around getting swagger code generation to work correctly, but of course any improvements that it could bring to Swagger UI is good too (see also #7446)

@careys7
Copy link
Member Author

careys7 commented Jan 30, 2018

@springimport the generator and the search criteria both work as expected. The OpenAPI specification (which is what the generator is producing) can't describe the searchCriteria implementation expected by Magento

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Feb 7, 2018
@magento-engcom-team
Copy link
Contributor

@careys7, thank you for your report.
We've acknowledged the issue and added to our backlog.

@springimport
Copy link

@careys7 This is sad.
My temporary (or permanent :) solution springimport/swagger-magento2-client@841c70d

@idziakjakub
Copy link
Contributor

#WroCD

@magento-engcom-team
Copy link
Contributor

@idziakjakub thank you for joining. Please accept team invitation here and self-assign the issue.

@ishakhsuvarov ishakhsuvarov removed Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line labels Jun 11, 2018
@techzilla
Copy link

Can also Confirm the issue is very much unfixed.

@ghost ghost assigned idziakjakub Apr 2, 2019
@careys7
Copy link
Member Author

careys7 commented Nov 26, 2019

Since reporting this issue, OpenAPI v3 has been released

OpenAPI v3 supports deep object notation which I think might be compatible with all of search criteria, so could be a possible option to describe the existing Magento WebAPI schema.

It would require either:

a) Generating both v2 & v3 schemas
b) Moving the schema to v3

@ghost ghost removed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Oct 20, 2020
@engcom-Bravo engcom-Bravo added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Nov 2, 2020
@sivaschenko sivaschenko added feature request Priority: P2 A defect with this priority could have functionality issues which are not to expectations. labels Nov 17, 2020
@davidhiendl
Copy link

davidhiendl commented Jul 21, 2021

This is still a major blocker when trying to use the API using a generated client (effectively you cannot use list endpoints in a typed language at all).

@samuelbsource
Copy link

Are there any updates on this?

@davidhiendl
Copy link

This is still an ongoing problem in addition to missing types for successful response bodies and various other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog feature request Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: dev in progress Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Status: In Progress
Development

No branches or pull requests