Follow up to #2940397: Provide x-definition to superseded x-entity-type

This added x-definition to tags but because it uses $ref this causes a problem if the entity type is not included in the definitions section.

\Drupal\openapi\OpenApiGenerator\OpenApiRestGenerator::getTags probably always should have been checking for $options['entity_type_id'] but since x-entity-type wasn't a OpenApi $ref it didn't cause a problem if the entity type was in the definitions section. Now it crashes swagger_ui because it references non-existing node.

CommentFileSizeAuthor
#3 2948198-3-TEST_ONLY.patch2.64 KBtedbow
#3 2948198-3b.patch4.61 KBtedbow
#2 2948198-2.patch786 bytestedbow
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Active » Needs review
FileSize
2948198-2.patch786 bytes

Here is a fix

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
FileSize
2948198-3b.patch4.61 KB
2948198-3-TEST_ONLY.patch2.64 KB

I forgot that we have ::includeEntityTypeBundle() which takes care of this.

Also adding tests to assert that all the tags in method section equals the tags section. To test this it now enables all REST resources for each request and ignores $options. This makes sure that if other resources are enabled but they are not in current request options they will not affect the returned spec.

Status: Needs review » Needs work

The last submitted patch, 3: 2948198-3-TEST_ONLY.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Needs work » Needs review

Forgot to upload 2948198-3-TEST_ONLY.patch first. Expected fail.

richgerdes’s picture

richgerdes
he/him
Primary language English
Location New Jersey, USA
CreditAttribution: richgerdes commented
Status: Needs review » Reviewed & tested by the community

My bad on not testing the docs modules. We need to make sure they have good test coverage to prevent this from happening. We may was to see if there is a php cli or rest tool that can be used to validate the schema as part of ci tests. That would also eliviate the need to validate the api schema by testing it against a cloud tool. we would then just have to verify that the correct information is generated for drupal to handle.

Marking this as RTBC. Its good to merge unless you come across other issues.

  • 28802bf committed on 8.x-1.x 
    Issue #2948198 by tedbow: x-definition in REST generated spec causes...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Reviewed & tested by the community » Fixed

My bad on not testing the docs modules.

No, problem there should have been basic test long before you got involved in with this project, my bad.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented

Created #2948564: Create tests for OpenApi Swagger UI module as a follow up

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.