magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.57k stars 9.32k forks source link

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

Open careys7 opened 7 years ago

careys7 commented 7 years ago

Re-opening #7511 per comment https://github.com/magento/magento2/issues/7511#issuecomment-336468740 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
dmanners commented 7 years ago

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 commented 7 years ago

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:

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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)

springimport commented 6 years ago

@careys7 I found the problem. https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Webapi/Model/Rest/Swagger/Generator.php#L719

careys7 commented 6 years ago

@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 commented 6 years ago

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

springimport commented 6 years ago

@careys7 This is sad. My temporary (or permanent :) solution https://github.com/springimport/swagger-magento2-client/commit/841c70d21982d10d37a1767fe2e9fa41f00c1375

idziakjakub commented 6 years ago

WroCD

magento-engcom-team commented 6 years ago

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

dmanners commented 6 years ago

Hi @idziakjakub thank you for the pull request. I will reopen this task and it will be closed once the pull request is accepted.

VladimirZaets commented 6 years ago

Hi @careys7. Thank you for your report. The issue has been fixed in magento/magento2#15322 by @idziakjakub in 2.2-develop branch Related commit(s):

The fix will be available with the upcoming 2.2.6 release.

careys7 commented 6 years ago

@VladimirZaets thanks, but it doesn't look like the issue is resolved

magento-engcom-team commented 6 years ago

Hi @careys7. Thank you for your report. The issue has been fixed in magento/magento2#15946 by @vgelani in 2.3-develop branch Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

sidolov commented 6 years ago

Hi @careys7. Thank you for your report. The issue has been fixed in magento/magento2#15945 by @vgelani in 2.1-develop branch Related commit(s):

The fix will be available with the upcoming 2.1.15 release.

careys7 commented 6 years ago

Please see comment

techzilla commented 6 years ago

Can also Confirm the issue is very much unfixed.

careys7 commented 5 years ago

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

davidhiendl commented 3 years ago

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 commented 3 years ago

Are there any updates on this?

davidhiendl commented 3 months ago

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