Skip to content

Commit

Permalink
importer-rest-api-specs: shelve SupplementaryData support for now, …
Browse files Browse the repository at this point in the history
…since it doesn't seem to be required for DataFactory models as they use external Swagger Refs.
  • Loading branch information
manicminer committed Jul 17, 2024
1 parent 924eae6 commit dfb8822
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
//}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down Expand Up @@ -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,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"swagger": "2.0",
"info": {
"title": "Example",
"title": "DataFactory",
"description": "Example",
"version": "2020-01-01"
},
Expand Down Expand Up @@ -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": {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -38,7 +52,7 @@
],
"title": "Cat",
"type": "object",
"x-ms-discriminator-value": "cat"
"x-ms-discriminator-value": "Cat"
}
},
"parameters": {}
Expand Down

0 comments on commit dfb8822

Please sign in to comment.