Skip to content

Commit 40b5be4

Browse files
bestbeforetodaybestbeforetoday
committed
Allow schema types with the same name from different packages
Previously the short type name was used as a schema key. This meant that types of the same name from different packages would overwrite each other in the map of schema type definitions, causing unexpected serialization errors. Now the fully qualified (package + type) name is used so that entries for types with the same name from different packages are unique. Signed-off-by: Mark S. Lewis <[email protected]>
1 parent fda7a56 commit 40b5be4

File tree

6 files changed

+114
-90
lines changed

6 files changed

+114
-90
lines changed

‎internal/functionaltests/contracts/complexcontract/contract-metadata/metadata.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
}
1818
],
1919
"returns": {
20-
"$ref": "#/components/schemas/BasicObject"
20+
"$ref": "#/components/schemas/github.com.hyperledger.fabric-contract-api-go.internal.functionaltests.contracts.complexcontract.BasicObject"
2121
},
2222
"tag": [
2323
"evaluate",
@@ -59,7 +59,7 @@
5959
{
6060
"name": "param1",
6161
"schema": {
62-
"$ref": "#/components/schemas/BasicOwner"
62+
"$ref": "#/components/schemas/github.com.hyperledger.fabric-contract-api-go.internal.functionaltests.contracts.complexcontract.BasicOwner"
6363
}
6464
},
6565
{
@@ -99,7 +99,7 @@
9999
{
100100
"name": "param1",
101101
"schema": {
102-
"$ref": "#/components/schemas/BasicOwner"
102+
"$ref": "#/components/schemas/github.com.hyperledger.fabric-contract-api-go.internal.functionaltests.contracts.complexcontract.BasicOwner"
103103
}
104104
}
105105
],

‎internal/types/types_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestBoolType(t *testing.T) {
7474
assert.False(t, val.Interface().(bool), "should have returned the boolean false for blank value")
7575

7676
val, err = boolTypeVar.Convert("non bool")
77-
assert.Error(t, fmt.Errorf(convertError, "non bool"), "should return error for invalid bool value")
77+
assert.Error(t, err, fmt.Errorf(convertError, "non bool"), "should return error for invalid bool value")
7878
assert.Equal(t, reflect.Value{}, val, "should have returned the blank value for non bool")
7979
}
8080

‎metadata/schema.go

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,32 +24,26 @@ func GetSchema(field reflect.Type, components *ComponentMetadata) (*spec.Schema,
2424
}
2525

2626
func getSchema(field reflect.Type, components *ComponentMetadata, nested bool) (*spec.Schema, error) {
27-
var schema *spec.Schema
28-
var err error
29-
30-
if bt, ok := types.BasicTypes[field.Kind()]; !ok {
31-
if field == types.TimeType {
32-
schema = spec.DateTimeProperty()
33-
} else if field.Kind() == reflect.Array {
34-
schema, err = buildArraySchema(reflect.New(field).Elem(), components, nested)
35-
} else if field.Kind() == reflect.Slice {
36-
schema, err = buildSliceSchema(reflect.MakeSlice(field, 1, 1), components, nested)
37-
} else if field.Kind() == reflect.Map {
38-
schema, err = buildMapSchema(reflect.MakeMap(field), components, nested)
39-
} else if field.Kind() == reflect.Struct || (field.Kind() == reflect.Ptr && field.Elem().Kind() == reflect.Struct) {
40-
schema, err = buildStructSchema(field, components, nested)
41-
} else {
42-
return nil, fmt.Errorf("%s was not a valid type", field.String())
43-
}
44-
} else {
27+
if bt, ok := types.BasicTypes[field.Kind()]; ok {
4528
return bt.GetSchema(), nil
4629
}
47-
48-
if err != nil {
49-
return nil, err
30+
if field == types.TimeType {
31+
return spec.DateTimeProperty(), nil
32+
}
33+
if field.Kind() == reflect.Array {
34+
return buildArraySchema(reflect.New(field).Elem(), components, nested)
35+
}
36+
if field.Kind() == reflect.Slice {
37+
return buildSliceSchema(reflect.MakeSlice(field, 1, 1), components, nested)
38+
}
39+
if field.Kind() == reflect.Map {
40+
return buildMapSchema(reflect.MakeMap(field), components, nested)
41+
}
42+
if field.Kind() == reflect.Struct || (field.Kind() == reflect.Ptr && field.Elem().Kind() == reflect.Struct) {
43+
return buildStructSchema(field, components, nested)
5044
}
5145

52-
return schema, nil
46+
return nil, fmt.Errorf("%s was not a valid type", field.String())
5347
}
5448

5549
func buildArraySchema(array reflect.Value, components *ComponentMetadata, nested bool) (*spec.Schema, error) {
@@ -95,12 +89,14 @@ func addComponentIfNotExists(obj reflect.Type, components *ComponentMetadata) er
9589
obj = obj.Elem()
9690
}
9791

98-
if _, ok := components.Schemas[obj.Name()]; ok {
92+
key := schemaKey(obj)
93+
94+
if _, ok := components.Schemas[key]; ok {
9995
return nil
10096
}
10197

10298
schema := ObjectMetadata{}
103-
schema.ID = obj.Name()
99+
schema.ID = key
104100
schema.Required = []string{}
105101
schema.Properties = make(map[string]spec.Schema)
106102
schema.AdditionalProperties = false
@@ -109,18 +105,18 @@ func addComponentIfNotExists(obj reflect.Type, components *ComponentMetadata) er
109105
components.Schemas = make(map[string]ObjectMetadata)
110106
}
111107

112-
components.Schemas[obj.Name()] = schema // lock up slot for cyclic
108+
components.Schemas[key] = schema // lock up slot for cyclic
113109

114110
for i := 0; i < obj.NumField(); i++ {
115111
err := getField(obj.Field(i), &schema, components)
116112

117113
if err != nil {
118-
delete(components.Schemas, obj.Name())
114+
delete(components.Schemas, key)
119115
return err
120116
}
121117
}
122118

123-
components.Schemas[obj.Name()] = schema // include changes
119+
components.Schemas[key] = schema // include changes
124120

125121
return nil
126122
}
@@ -205,5 +201,10 @@ func buildStructSchema(obj reflect.Type, components *ComponentMetadata, nested b
205201
refPath = ""
206202
}
207203

208-
return spec.RefSchema(refPath + obj.Name()), nil
204+
return spec.RefSchema(refPath + schemaKey(obj)), nil
205+
}
206+
207+
func schemaKey(obj reflect.Type) string {
208+
return strings.ReplaceAll(obj.PkgPath(), "/", ".") + "." + obj.Name()
209+
// return obj.Name()
209210
}

‎metadata/schema_test.go

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ var simpleStructPropertiesMap = map[string]spec.Schema{
4545
"jsonname2": *spec.StringProperty(),
4646
}
4747

48+
var simpleStructKey = schemaKey(reflect.TypeOf(simpleStruct{}))
49+
4850
var simpleStructMetadata = ObjectMetadata{
49-
ID: "simpleStruct",
51+
ID: simpleStructKey,
5052
Properties: simpleStructPropertiesMap,
5153
Required: []string{"Prop1", "propname", "Prop5", "jsonname2"},
5254
AdditionalProperties: false,
@@ -59,15 +61,17 @@ type complexStruct struct {
5961
Prop3 *complexStruct `metadata:",optional"`
6062
}
6163

64+
var complexStructKey = schemaKey(reflect.TypeOf(complexStruct{}))
65+
6266
var complexStructPropertiesMap = map[string]spec.Schema{
6367
"Prop0": *spec.StringProperty(),
6468
"Prop1": *spec.StringProperty(),
65-
"Prop2": *spec.RefSchema("simpleStruct"),
66-
"Prop3": *spec.RefSchema("complexStruct"),
69+
"Prop2": *spec.RefSchema(simpleStructKey),
70+
"Prop3": *spec.RefSchema(complexStructKey),
6771
}
6872

6973
var complexStructMetadata = ObjectMetadata{
70-
ID: "complexStruct",
74+
ID: complexStructKey,
7175
Properties: complexStructPropertiesMap,
7276
Required: []string{"Prop0", "Prop1", "Prop2"},
7377
AdditionalProperties: false,
@@ -84,16 +88,18 @@ type superComplexStruct struct {
8488
var superComplexStructPropertiesMap = map[string]spec.Schema{
8589
"Prop0": *spec.StringProperty(),
8690
"Prop1": *spec.StringProperty(),
87-
"Prop2": *spec.RefSchema("simpleStruct"),
88-
"Prop3": *spec.RefSchema("complexStruct"),
89-
"Prop4": *spec.ArrayProperty(spec.RefSchema("complexStruct")),
90-
"Prop5": *spec.ArrayProperty(spec.RefSchema("simpleStruct")),
91-
"Prop6": *spec.MapProperty(spec.RefSchema("complexStruct")),
92-
"Prop7": *spec.MapProperty(spec.ArrayProperty(spec.RefSchema("simpleStruct"))),
91+
"Prop2": *spec.RefSchema(simpleStructKey),
92+
"Prop3": *spec.RefSchema(complexStructKey),
93+
"Prop4": *spec.ArrayProperty(spec.RefSchema(complexStructKey)),
94+
"Prop5": *spec.ArrayProperty(spec.RefSchema(simpleStructKey)),
95+
"Prop6": *spec.MapProperty(spec.RefSchema(complexStructKey)),
96+
"Prop7": *spec.MapProperty(spec.ArrayProperty(spec.RefSchema(simpleStructKey))),
9397
}
9498

99+
var superComplexStructKey = schemaKey(reflect.TypeOf(superComplexStruct{}))
100+
95101
var superComplexStructMetadata = ObjectMetadata{
96-
ID: "superComplexStruct",
102+
ID: superComplexStructKey,
97103
Properties: superComplexStructPropertiesMap,
98104
Required: append(complexStructMetadata.Required, "Prop4", "Prop5", "Prop6", "Prop7"),
99105
AdditionalProperties: false,
@@ -201,18 +207,18 @@ func TestAddComponentIfNotExists(t *testing.T) {
201207

202208
components = new(ComponentMetadata)
203209
components.Schemas = make(map[string]ObjectMetadata)
204-
components.Schemas["simpleStruct"] = someObject
210+
components.Schemas[simpleStructKey] = someObject
205211

206212
err = addComponentIfNotExists(reflect.TypeOf(simpleStruct{}), components)
207-
assert.Nil(t, err, "should return nil for error when component of name already exists")
213+
assert.NoError(t, err, "should return nil for error when component of name already exists")
208214
assert.Equal(t, len(components.Schemas), 1, "should not have added a new component when one already exists")
209-
_, ok = components.Schemas["simpleStruct"].Properties["some property"]
215+
_, ok = components.Schemas[simpleStructKey].Properties["some property"]
210216
assert.True(t, ok, "should not overwrite existing component")
211217

212218
err = addComponentIfNotExists(reflect.TypeOf(new(simpleStruct)), components)
213219
assert.Nil(t, err, "should return nil for error when component of name already exists for pointer")
214220
assert.Equal(t, len(components.Schemas), 1, "should not have added a new component when one already exists for pointer")
215-
_, ok = components.Schemas["simpleStruct"].Properties["some property"]
221+
_, ok = components.Schemas[simpleStructKey].Properties["some property"]
216222
assert.True(t, ok, "should not overwrite existing component when already exists and pointer passed")
217223

218224
err = addComponentIfNotExists(reflect.TypeOf(badStruct{}), components)
@@ -222,13 +228,13 @@ func TestAddComponentIfNotExists(t *testing.T) {
222228
components.Schemas = nil
223229
err = addComponentIfNotExists(reflect.TypeOf(simpleStruct{}), components)
224230
assert.Nil(t, err, "should not error when adding new component when schemas not initialised")
225-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should set correct metadata for new component when schemas not initialised")
231+
assert.Equal(t, components.Schemas[simpleStructKey], simpleStructMetadata, "should set correct metadata for new component when schemas not initialised")
226232

227-
delete(components.Schemas, "simpleStruct")
233+
delete(components.Schemas, simpleStructKey)
228234
components.Schemas["otherStruct"] = someObject
229235
err = addComponentIfNotExists(reflect.TypeOf(simpleStruct{}), components)
230236
assert.Nil(t, err, "should not error when adding new component")
231-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should set correct metadata for new component")
237+
assert.Equal(t, components.Schemas[simpleStructKey], simpleStructMetadata, "should set correct metadata for new component")
232238
assert.Equal(t, components.Schemas["otherStruct"], someObject, "should not affect existing components")
233239
}
234240

@@ -247,21 +253,21 @@ func TestBuildStructSchema(t *testing.T) {
247253

248254
schema, err = buildStructSchema(reflect.TypeOf(simpleStruct{}), components, false)
249255
assert.Nil(t, err, "should not return error when struct is good")
250-
assert.Equal(t, schema, spec.RefSchema("#/components/schemas/simpleStruct"), "should make schema ref to component")
251-
_, ok := components.Schemas["simpleStruct"]
256+
assert.Equal(t, spec.RefSchema("#/components/schemas/"+simpleStructKey), schema, "should make schema ref to component")
257+
_, ok := components.Schemas[simpleStructKey]
252258
assert.True(t, ok, "should have added component")
253259

254260
schema, err = buildStructSchema(reflect.TypeOf(simpleStruct{}), components, true)
255261
assert.Nil(t, err, "should not return error when struct is good")
256-
assert.Equal(t, schema, spec.RefSchema("simpleStruct"), "should make schema ref to component for nested ref")
257-
_, ok = components.Schemas["simpleStruct"]
262+
assert.Equal(t, spec.RefSchema(simpleStructKey), schema, "should make schema ref to component for nested ref")
263+
_, ok = components.Schemas[simpleStructKey]
258264
assert.True(t, ok, "should have added component for nested ref")
259265

260266
schema, err = buildStructSchema(reflect.TypeOf(new(simpleStruct)), components, false)
261267
assert.Nil(t, err, "should not return error when pointer to struct is good")
262-
assert.Equal(t, schema, spec.RefSchema("#/components/schemas/simpleStruct"), "should make schema ref to component")
268+
assert.Equal(t, spec.RefSchema("#/components/schemas/"+simpleStructKey), schema, "should make schema ref to component")
263269

264-
_, ok = components.Schemas["simpleStruct"]
270+
_, ok = components.Schemas[simpleStructKey]
265271
assert.True(t, ok, "should have use already added component")
266272
}
267273

@@ -444,9 +450,9 @@ func TestGetSchema(t *testing.T) {
444450
schema, err = GetSchema(reflect.TypeOf(simpleStruct{}), components)
445451

446452
assert.Nil(t, err, "should return nil when valid object")
447-
assert.Equal(t, len(components.Schemas), 1, "should have added a new component")
448-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should have added correct metadata to components")
449-
assert.Equal(t, schema, spec.RefSchema("#/components/schemas/simpleStruct"))
453+
assert.Equal(t, 1, len(components.Schemas), "should have added a new component")
454+
assert.Equal(t, simpleStructMetadata, components.Schemas[simpleStructKey], "should have added correct metadata to components")
455+
assert.Equal(t, spec.RefSchema("#/components/schemas/"+simpleStructKey), schema)
450456

451457
// should handle pointer to struct
452458
components = new(ComponentMetadata)
@@ -455,9 +461,9 @@ func TestGetSchema(t *testing.T) {
455461
schema, err = GetSchema(reflect.TypeOf(new(simpleStruct)), components)
456462

457463
assert.Nil(t, err, "should return nil when valid object")
458-
assert.Equal(t, len(components.Schemas), 1, "should have added a new component")
459-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should have added correct metadata to components")
460-
assert.Equal(t, schema, spec.RefSchema("#/components/schemas/simpleStruct"))
464+
assert.Equal(t, 1, len(components.Schemas), "should have added a new component")
465+
assert.Equal(t, simpleStructMetadata, components.Schemas[simpleStructKey], "should have added correct metadata to components")
466+
assert.Equal(t, spec.RefSchema("#/components/schemas/"+simpleStructKey), schema)
461467

462468
// Should handle an array of structs
463469
components = new(ComponentMetadata)
@@ -466,9 +472,9 @@ func TestGetSchema(t *testing.T) {
466472
schema, err = GetSchema(reflect.TypeOf([1]simpleStruct{}), components)
467473

468474
assert.Nil(t, err, "should return nil when valid object")
469-
assert.Equal(t, len(components.Schemas), 1, "should have added a new component")
470-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should have added correct metadata to components")
471-
assert.Equal(t, schema, spec.ArrayProperty(spec.RefSchema("#/components/schemas/simpleStruct")))
475+
assert.Equal(t, 1, len(components.Schemas), "should have added a new component")
476+
assert.Equal(t, simpleStructMetadata, components.Schemas[simpleStructKey], "should have added correct metadata to components")
477+
assert.Equal(t, spec.ArrayProperty(spec.RefSchema("#/components/schemas/"+simpleStructKey)), schema)
472478

473479
// Should handle a slice of structs
474480
components = new(ComponentMetadata)
@@ -477,9 +483,9 @@ func TestGetSchema(t *testing.T) {
477483
schema, err = GetSchema(reflect.TypeOf([]simpleStruct{}), components)
478484

479485
assert.Nil(t, err, "should return nil when valid object")
480-
assert.Equal(t, len(components.Schemas), 1, "should have added a new component")
481-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should have added correct metadata to components")
482-
assert.Equal(t, schema, spec.ArrayProperty(spec.RefSchema("#/components/schemas/simpleStruct")))
486+
assert.Equal(t, 1, len(components.Schemas), "should have added a new component")
487+
assert.Equal(t, simpleStructMetadata, components.Schemas[simpleStructKey], "should have added correct metadata to components")
488+
assert.Equal(t, spec.ArrayProperty(spec.RefSchema("#/components/schemas/"+simpleStructKey)), schema)
483489

484490
// Should handle a valid struct with struct property and add to components
485491
components = new(ComponentMetadata)
@@ -488,10 +494,10 @@ func TestGetSchema(t *testing.T) {
488494
schema, err = GetSchema(reflect.TypeOf(new(complexStruct)), components)
489495

490496
assert.Nil(t, err, "should return nil when valid object")
491-
assert.Equal(t, len(components.Schemas), 2, "should have added two new components")
492-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should have added correct metadata to components for sub struct")
493-
assert.Equal(t, components.Schemas["complexStruct"], complexStructMetadata, "should have added correct metadata to components for main struct")
494-
assert.Equal(t, schema, spec.RefSchema("#/components/schemas/complexStruct"))
497+
assert.Equal(t, 2, len(components.Schemas), "should have added two new components")
498+
assert.Equal(t, simpleStructMetadata, components.Schemas[simpleStructKey], "should have added correct metadata to components for sub struct")
499+
assert.Equal(t, complexStructMetadata, components.Schemas[complexStructKey], "should have added correct metadata to components for main struct")
500+
assert.Equal(t, spec.RefSchema("#/components/schemas/"+complexStructKey), schema)
495501

496502
// Should handle a valid struct with struct properties of array, slice and map types
497503
components = new(ComponentMetadata)
@@ -500,11 +506,11 @@ func TestGetSchema(t *testing.T) {
500506
schema, err = GetSchema(reflect.TypeOf(new(superComplexStruct)), components)
501507

502508
assert.Nil(t, err, "should return nil when valid object")
503-
assert.Equal(t, len(components.Schemas), 3, "should have added two new components")
504-
assert.Equal(t, components.Schemas["simpleStruct"], simpleStructMetadata, "should have added correct metadata to components for sub struct")
505-
assert.Equal(t, components.Schemas["complexStruct"], complexStructMetadata, "should have added correct metadata to components for sub struct")
506-
assert.Equal(t, components.Schemas["superComplexStruct"], superComplexStructMetadata, "should have added correct metadata to components for main struct")
507-
assert.Equal(t, schema, spec.RefSchema("#/components/schemas/superComplexStruct"))
509+
assert.Equal(t, 3, len(components.Schemas), "should have added two new components")
510+
assert.Equal(t, simpleStructMetadata, components.Schemas[simpleStructKey], "should have added correct metadata to components for sub struct")
511+
assert.Equal(t, complexStructMetadata, components.Schemas[complexStructKey], "should have added correct metadata to components for sub struct")
512+
assert.Equal(t, superComplexStructMetadata, components.Schemas[superComplexStructKey], "should have added correct metadata to components for main struct")
513+
assert.Equal(t, spec.RefSchema("#/components/schemas/"+superComplexStructKey), schema)
508514

509515
// Should return an error for a bad struct
510516
components = new(ComponentMetadata)

‎serializer/internal/test_data.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Copyright the Hyperledger Fabric contributors. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package internal
5+
6+
type SimpleStruct struct {
7+
Prop1 string `json:"Prop1"`
8+
}

0 commit comments

Comments
 (0)