Skip to content

Commit 28389a7

Browse files
authored
Add support for objects with properties that omit the type field (#80)
* add negative test * add fix for empty object type * add changelog
1 parent b9226bb commit 28389a7

File tree

6 files changed

+143
-5
lines changed

6 files changed

+143
-5
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: ENHANCEMENTS
2+
body: Schemas that have the `properties` keyword defined with no type will now default
3+
to `object`
4+
time: 2023-10-20T17:02:09.205328-04:00
5+
custom:
6+
Issue: "79"

internal/cmd/testdata/edgecase/generator_config.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,8 @@ data_sources:
3030
map_test:
3131
read:
3232
path: /map_test
33+
method: GET
34+
obj_no_type:
35+
read:
36+
path: /obj_no_type
3337
method: GET

internal/cmd/testdata/edgecase/openapi_spec.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@ info:
44
description: This is a fake API spec that was built to test some of the less common API schema structures and their mapping in the OpenAPI to Framework code generator
55
version: 1.0.0
66
paths:
7+
/obj_no_type:
8+
get:
9+
summary: Test for objects that have no types!
10+
responses:
11+
"200":
12+
description: OK
13+
content:
14+
application/json:
15+
schema:
16+
properties:
17+
string_prop:
18+
description: String inside an object!
19+
type: string
20+
nested_obj:
21+
properties:
22+
bool_prop:
23+
description: Bool inside a nested object!
24+
type: boolean
725
/nested_collections:
826
get:
927
summary: Test for nested collections (list within a list, set within a list, map within a list)

internal/cmd/testdata/edgecase/provider_code_spec.json

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,35 @@
120120
]
121121
}
122122
},
123+
{
124+
"name": "obj_no_type",
125+
"schema": {
126+
"attributes": [
127+
{
128+
"name": "nested_obj",
129+
"single_nested": {
130+
"computed_optional_required": "computed",
131+
"attributes": [
132+
{
133+
"name": "bool_prop",
134+
"bool": {
135+
"computed_optional_required": "computed",
136+
"description": "Bool inside a nested object!"
137+
}
138+
}
139+
]
140+
}
141+
},
142+
{
143+
"name": "string_prop",
144+
"string": {
145+
"computed_optional_required": "computed",
146+
"description": "String inside an object!"
147+
}
148+
}
149+
]
150+
}
151+
},
123152
{
124153
"name": "set_test",
125154
"schema": {

internal/mapper/oas/build.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,12 @@ func getMultiTypeSchema(proxyOne *base.SchemaProxy, proxyTwo *base.SchemaProxy)
224224
func retrieveType(schema *base.Schema) (string, *SchemaError) {
225225
switch len(schema.Type) {
226226
case 0:
227+
// Properties are only valid applying to objects, it's possible tools might omit the type
228+
// https://github.com/hashicorp/terraform-plugin-codegen-openapi/issues/79
229+
if len(schema.Properties) > 0 {
230+
return util.OAS_type_object, nil
231+
}
232+
227233
return "", SchemaErrorFromProxy(errors.New("no 'type' array or supported allOf, oneOf, anyOf constraint - attribute cannot be created"), schema.ParentProxy)
228234
case 1:
229235
return schema.Type[0], nil

internal/mapper/oas/build_test.go

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import (
1818
high "github.com/pb33f/libopenapi/datamodel/high/v3"
1919
)
2020

21-
// TODO: write tests for error paths (nullable types + build schema functions)
22-
2321
func TestBuildSchemaFromRequest(t *testing.T) {
2422
t.Parallel()
2523

@@ -126,7 +124,6 @@ func TestBuildSchemaFromRequest(t *testing.T) {
126124
}
127125
})
128126
}
129-
130127
}
131128

132129
func TestBuildSchemaFromRequest_Errors(t *testing.T) {
@@ -337,7 +334,6 @@ func TestBuildSchemaFromResponse(t *testing.T) {
337334
}
338335
})
339336
}
340-
341337
}
342338

343339
func TestBuildSchemaFromResponse_Errors(t *testing.T) {
@@ -469,7 +465,6 @@ func TestBuildSchemaFromResponse_Errors(t *testing.T) {
469465
}
470466
})
471467
}
472-
473468
}
474469

475470
func TestBuildSchema_MultiTypes(t *testing.T) {
@@ -1209,3 +1204,83 @@ func TestBuildSchema_Errors(t *testing.T) {
12091204
})
12101205
}
12111206
}
1207+
1208+
func TestBuildSchema_EdgeCases(t *testing.T) {
1209+
t.Parallel()
1210+
1211+
testCases := map[string]struct {
1212+
schemaProxy *base.SchemaProxy
1213+
expectedAttributes attrmapper.ResourceAttributes
1214+
}{
1215+
"no type with properties - defaults to object": {
1216+
schemaProxy: base.CreateSchemaProxy(&base.Schema{
1217+
Type: []string{},
1218+
Required: []string{"string", "object"},
1219+
Properties: map[string]*base.SchemaProxy{
1220+
"string": base.CreateSchemaProxy(&base.Schema{
1221+
Type: []string{"string"},
1222+
Description: "hey there! I'm a string type, required.",
1223+
}),
1224+
"object": base.CreateSchemaProxy(&base.Schema{
1225+
Type: []string{},
1226+
Required: []string{"bool"},
1227+
Description: "hey there! I'm an object type, required.",
1228+
Properties: map[string]*base.SchemaProxy{
1229+
"bool": base.CreateSchemaProxy(&base.Schema{
1230+
Type: []string{"boolean"},
1231+
Description: "hey there! I'm a bool type, required.",
1232+
}),
1233+
},
1234+
}),
1235+
},
1236+
}),
1237+
expectedAttributes: attrmapper.ResourceAttributes{
1238+
&attrmapper.ResourceSingleNestedAttribute{
1239+
Name: "object",
1240+
Attributes: attrmapper.ResourceAttributes{
1241+
&attrmapper.ResourceBoolAttribute{
1242+
Name: "bool",
1243+
BoolAttribute: resource.BoolAttribute{
1244+
ComputedOptionalRequired: schema.Required,
1245+
Description: pointer("hey there! I'm a bool type, required."),
1246+
},
1247+
},
1248+
},
1249+
SingleNestedAttribute: resource.SingleNestedAttribute{
1250+
ComputedOptionalRequired: schema.Required,
1251+
Description: pointer("hey there! I'm an object type, required."),
1252+
},
1253+
},
1254+
&attrmapper.ResourceStringAttribute{
1255+
Name: "string",
1256+
StringAttribute: resource.StringAttribute{
1257+
ComputedOptionalRequired: schema.Required,
1258+
Description: pointer("hey there! I'm a string type, required."),
1259+
},
1260+
},
1261+
},
1262+
},
1263+
}
1264+
1265+
for name, testCase := range testCases {
1266+
name, testCase := name, testCase
1267+
1268+
t.Run(name, func(t *testing.T) {
1269+
t.Parallel()
1270+
1271+
schema, err := oas.BuildSchema(testCase.schemaProxy, oas.SchemaOpts{}, oas.GlobalSchemaOpts{})
1272+
if err != nil {
1273+
t.Fatalf("unexpected error: %s", err)
1274+
}
1275+
1276+
attributes, err := schema.BuildResourceAttributes()
1277+
if err != nil {
1278+
t.Fatalf("unexpected error: %s", err)
1279+
}
1280+
1281+
if diff := cmp.Diff(attributes, testCase.expectedAttributes); diff != "" {
1282+
t.Errorf("unexpected difference: %s", diff)
1283+
}
1284+
})
1285+
}
1286+
}

0 commit comments

Comments
 (0)