From dfb8822b333ce4f2ffcb0a1221ee0c8f0c19175a Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 17 Jul 2024 11:51:24 +0100 Subject: [PATCH] `importer-rest-api-specs`: shelve SupplementaryData support for now, since it doesn't seem to be required for DataFactory models as they use external Swagger Refs. --- .../apidefinitions/parse_api_version.go | 2 +- .../apidefinitions/parse_service.go | 2 +- .../parse_supplementary_data.go | 38 ++++------ .../parser/models_discriminators_test.go | 75 +++++++++++++++++++ .../parser/parse_supplementary_data.go | 6 +- .../parser/parse_supplementary_data_test.go | 57 -------------- ...discriminators_ref_from_another_spec.json} | 17 +---- ...l_discriminators_ref_in_another_spec.json} | 18 ++++- 8 files changed, 116 insertions(+), 99 deletions(-) delete mode 100644 tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data_test.go rename tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/{supplementary_data_parent.json => model_discriminators_ref_from_another_spec.json} (70%) rename tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/{supplementary_data_implementations.json => model_discriminators_ref_in_another_spec.json} (64%) diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_api_version.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_api_version.go index de2229f2e3d..d3c49b5310c 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_api_version.go +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_api_version.go @@ -45,7 +45,7 @@ func parseAPIVersion(serviceName string, input discoveryModels.AvailableDataSetF logging.Tracef("Processing API Definitions from file %q..", filePath) resources, err := parseAPIResourcesFromFile(filePath, serviceName, resourceProvider, apiResources, foundResourceIDs) if err != nil { - return nil, fmt.Errorf("parsing the APIResources from the Supplementary Data within %q: %+v", filePath, err) + return nil, fmt.Errorf("parsing the APIResources from the API Definitions within %q: %+v", filePath, err) } logging.Tracef("There are now %d APIResources", len(*resources)) diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_service.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_service.go index a4fd32be542..0a460356a12 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_service.go +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_service.go @@ -2,9 +2,9 @@ package apidefinitions import ( "fmt" - "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/ignore" sdkModels "github.com/hashicorp/pandora/tools/data-api-sdk/v1/models" + "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/ignore" discoveryModels "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/discovery/models" "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/logging" ) diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_supplementary_data.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_supplementary_data.go index 2eac14db824..64aed08e76d 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_supplementary_data.go +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parse_supplementary_data.go @@ -1,26 +1,18 @@ package apidefinitions -import ( - "fmt" - - "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/apidefinitions/parser" - parserModels "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/models" - "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/logging" -) - // parseSupplementaryDataFromFile loads any available Supplementary Data from the file in question -func parseSupplementaryDataFromFile(filePath string) (*parserModels.SupplementaryData, error) { - logging.Tracef("Building an Definitions Parser for %q..", filePath) - parser, err := parser.NewAPIDefinitionsParser(filePath) - if err != nil { - return nil, fmt.Errorf("building the APIDefinitions Parser for %q: %+v", filePath, err) - } - logging.Tracef("Parsing the Supplementary Data from %q..", filePath) - data, err := parser.SupplementaryData() - if err != nil { - return nil, fmt.Errorf("parsing the Supplementary Data from %q: %+v", filePath, err) - } - logging.Tracef("Loaded the Supplementary Data from %q.", filePath) - - return data, nil -} +//func parseSupplementaryDataFromFile(filePath string) (*parserModels.SupplementaryData, error) { +// logging.Tracef("Building an Definitions Parser for %q..", filePath) +// parser, err := parser.NewAPIDefinitionsParser(filePath) +// if err != nil { +// return nil, fmt.Errorf("building the APIDefinitions Parser for %q: %+v", filePath, err) +// } +// logging.Tracef("Parsing the Supplementary Data from %q..", filePath) +// data, err := parser.SupplementaryData() +// if err != nil { +// return nil, fmt.Errorf("parsing the Supplementary Data from %q: %+v", filePath, err) +// } +// logging.Tracef("Loaded the Supplementary Data from %q.", filePath) +// +// return data, nil +//} diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/models_discriminators_test.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/models_discriminators_test.go index ae496c23c7d..af1aa4511ef 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/models_discriminators_test.go +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/models_discriminators_test.go @@ -1411,6 +1411,81 @@ func TestParseDiscriminatorsOrphanedChildWithoutDiscriminatorValue(t *testing.T) testhelpers.ValidateParsedSwaggerResultMatches(t, expected, actual) } +func TestParseDiscriminatorsReferenceInAnotherSpecFile(t *testing.T) { + // This tests whether Swagger Refs from another spec file are parsed correctly. The go-openapi module actually does + // the heaving lifting here (thankfully!), loading in another file transparently when an external Ref is encountered. + // An external Ref looks something like this: + // + // "$ref": "./model_discriminators_ref_in_another_spec.json#/definitions/Animal" + // + // Where the anchor is preceded by a path to a neighboring spec. We don't need to code for this explicitly, nor + // do we _really_ need to test for this, but since DataFactory relies heavily on this, this helps ensure we don't + // break this in the future. + actual, err := testhelpers.ParseSwaggerFileForTesting(t, "model_discriminators_ref_from_another_spec.json", pointer.To("DataFactory")) + if err != nil { + t.Fatalf("parsing: %+v", err) + } + + expected := sdkModels.APIVersion{ + APIVersion: "2020-01-01", + Resources: map[string]sdkModels.APIResource{ + "Example": { + Constants: make(map[string]sdkModels.SDKConstant), + Models: map[string]sdkModels.SDKModel{ + "Animal": { + Fields: map[string]sdkModels.SDKField{ + "AnimalType": { + JsonName: "animalType", + ObjectDefinition: sdkModels.SDKObjectDefinition{ + Type: sdkModels.StringSDKObjectDefinitionType, + }, + Required: true, + }, + }, + FieldNameContainingDiscriminatedValue: pointer.To("AnimalType"), + }, + "Cat": { + Fields: map[string]sdkModels.SDKField{ + "AnimalType": { + JsonName: "animalType", + ObjectDefinition: sdkModels.SDKObjectDefinition{ + Type: sdkModels.StringSDKObjectDefinitionType, + }, + Required: true, + }, + "IsFluffy": { + JsonName: "isFluffy", + ObjectDefinition: sdkModels.SDKObjectDefinition{ + Type: sdkModels.BooleanSDKObjectDefinitionType, + }, + Required: true, + }, + }, + ParentTypeName: pointer.To("Animal"), + FieldNameContainingDiscriminatedValue: pointer.To("AnimalType"), + DiscriminatedValue: pointer.To("Cat"), + }, + }, + Name: "Example", + Operations: map[string]sdkModels.SDKOperation{ + "Test": { + ContentType: "application/json", + ExpectedStatusCodes: []int{200}, + Method: "GET", + URISuffix: pointer.To("/example"), + ResponseObject: &sdkModels.SDKObjectDefinition{ + Type: sdkModels.ReferenceSDKObjectDefinitionType, + ReferenceName: pointer.To("Animal"), + }, + }, + }, + ResourceIDs: map[string]sdkModels.ResourceID{}, + }, + }, + } + testhelpers.ValidateParsedSwaggerResultMatches(t, expected, actual) +} + func TestParseDiscriminatorsOrphanedChildWithoutDiscriminatorValueForDifferentService(t *testing.T) { actual, err := testhelpers.ParseSwaggerFileForTesting(t, "/nestedtestdata/model_discriminators_orphaned_child_without_discriminator_value.json", pointer.To("Compute")) if err != nil { diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data.go index 37630c8b83b..f048ce280c7 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data.go +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data.go @@ -8,6 +8,10 @@ import ( parserModels "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/models" ) +// Deprecated: SupplementaryData is unused at this time, but being left in as a potential requirement. Note that this +// doesn't seem to be needed at this time for DataFactory, which uses external swagger refs - since support for this is +// built into the go-openapi module. +// TODO: @manicminer revisit and determine whether we'll implement this func (p *apiDefinitionsParser) SupplementaryData() (*parserModels.SupplementaryData, error) { result := parserModels.SupplementaryData{ Constants: map[string]sdkModels.SDKConstant{}, @@ -36,7 +40,7 @@ func (p *apiDefinitionsParser) SupplementaryData() (*parserModels.SupplementaryD } } - // FindNestedItemsYetToBeParsed takes ParseResult and so we need to shim this across + // FindNestedItemsYetToBeParsed takes ParseResult, so we need to shim this across shim := parserModels.ParseResult{ Constants: result.Constants, Models: result.Models, diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data_test.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data_test.go deleted file mode 100644 index fb6645cf351..00000000000 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parse_supplementary_data_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package parser_test - -import ( - "path/filepath" - "testing" - - "github.com/hashicorp/go-azure-helpers/lang/pointer" - sdkModels "github.com/hashicorp/pandora/tools/data-api-sdk/v1/models" - "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testhelpers" - discoveryModels "github.com/hashicorp/pandora/tools/importer-rest-api-specs/internal/components/discovery/models" -) - -func TestParseSupplementaryData(t *testing.T) { - dataSet := discoveryModels.AvailableDataSet{ - ServiceName: "Example", - DataSetsForAPIVersions: map[string]discoveryModels.AvailableDataSetForAPIVersion{ - "2020-01-01": { - APIVersion: "2020-01-01", - ContainsStableAPIVersion: true, - FilePathsContainingAPIDefinitions: []string{ - filepath.Join("testdata", "supplementary_data_parent.json"), - }, - FilePathsContainingSupplementaryData: []string{ - filepath.Join("testdata", "supplementary_data_implementations.json"), - }, - }, - }, - ResourceProvider: nil, - } - actual, err := testhelpers.ParseDataSetForTesting(t, dataSet, "2020-01-01") - if err != nil { - t.Fatalf(err.Error()) - } - expected := sdkModels.APIVersion{ - APIVersion: "2020-01-01", - Resources: map[string]sdkModels.APIResource{ - "Example": { - Constants: make(map[string]sdkModels.SDKConstant), - Models: map[string]sdkModels.SDKModel{ - "Cat": {}, - "ParentType": {}, - }, - Name: "Example", - Operations: map[string]sdkModels.SDKOperation{ - "Test": { - ResponseObject: &sdkModels.SDKObjectDefinition{ - Type: sdkModels.ReferenceSDKObjectDefinitionType, - ReferenceName: pointer.To("ParentType"), - }, - }, - }, - ResourceIDs: map[string]sdkModels.ResourceID{}, - }, - }, - } - testhelpers.ValidateParsedSwaggerResultMatches(t, expected, actual) -} diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/supplementary_data_parent.json b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/model_discriminators_ref_from_another_spec.json similarity index 70% rename from tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/supplementary_data_parent.json rename to tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/model_discriminators_ref_from_another_spec.json index 8fa4340aae7..5c45ec0e49e 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/supplementary_data_parent.json +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/model_discriminators_ref_from_another_spec.json @@ -1,7 +1,7 @@ { "swagger": "2.0", "info": { - "title": "Example", + "title": "DataFactory", "description": "Example", "version": "2020-01-01" }, @@ -30,24 +30,13 @@ "200": { "description": "Success.", "schema": { - "$ref": "#/definitions/ParentType" + "$ref": "./model_discriminators_ref_in_another_spec.json#/definitions/Animal" } } } } } }, - "definitions": { - "ParentType": { - "discriminator": "typeName", - "properties": { - "typeName": { - "type": "string" - } - }, - "type": "object", - "title": "ParentType" - } - }, + "definitions": {}, "parameters": {} } \ No newline at end of file diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/supplementary_data_implementations.json b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/model_discriminators_ref_in_another_spec.json similarity index 64% rename from tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/supplementary_data_implementations.json rename to tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/model_discriminators_ref_in_another_spec.json index ae7e1f676c7..4a5b6db19e7 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/supplementary_data_implementations.json +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/testdata/model_discriminators_ref_in_another_spec.json @@ -19,11 +19,25 @@ "securityDefinitions": {}, "paths": {}, "definitions": { + "Animal": { + "discriminator": "animalType", + "properties": { + "animalType": { + "type": "string", + "description": "The type of Animal this is, used as a Discriminator value." + } + }, + "required": [ + "animalType" + ], + "title": "Animal", + "type": "object" + }, "Cat": { "description": "A cat is a kind of ParentType", "allOf": [ { - "$ref": "#/definitions/ParentType" + "$ref": "#/definitions/Animal" } ], "properties": { @@ -38,7 +52,7 @@ ], "title": "Cat", "type": "object", - "x-ms-discriminator-value": "cat" + "x-ms-discriminator-value": "Cat" } }, "parameters": {}