Skip to content

Commit

Permalink
Fix non-deterministic behavior in generating schema (#104)
Browse files Browse the repository at this point in the history
**CONTAINS BREAKING CHANGES**

* Fix bug that caused non-deterministic behavior in generating functions for GET requests

* BREAKING CHANGE! Get rid of using Title for resource names. Require operationId.

* Add a test for discriminated type resources

* Fix bug with simple property refs and add test for it

* Reorganize the code for readability

* Fix bug that caused non-determinism for allOf definitions under certain conditions

* Don't skip non-object type allOf definitions that have yet another allOf definition

* Add CSharp namespaces for all modules and not just for certain request methods

* Generate language-specific overrides for non-camel-cased property names

* Add tests for naming funcs

* Upgrade deps and fix upgrade errors

* Normalize prop names (#117)

* Ensure property names in Pulumi schema are always camelCase

* Update ToSdkName to use toCamelInitCase

* Add name override for C# if the property name is the same as the enclosing type

* Exclude id property from resource input/output when handling allOf definitions
  • Loading branch information
praneetloke authored Mar 10, 2024
1 parent 551fb45 commit ae0c6a2
Show file tree
Hide file tree
Showing 17 changed files with 1,973 additions and 3,493 deletions.
247 changes: 117 additions & 130 deletions go.mod

Large diffs are not rendered by default.

3,240 changes: 283 additions & 2,957 deletions go.sum

Large diffs are not rendered by default.

41 changes: 41 additions & 0 deletions pkg/bug_103_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package pkg

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
)

func TestBug103(t *testing.T) {
mustReadTestOpenAPIDoc(t, filepath.Join("testdata", "bug_103_openapi.yml"))

openAPICtx := &OpenAPIContext{
Doc: *testOpenAPIDoc,
Pkg: &testPulumiPkg,
}

csharpNamespaces := map[string]string{
"": "Provider",
}

providerMetadata, _, err := openAPICtx.GatherResourcesFromAPI(csharpNamespaces)
assert.Nil(t, err)
assert.NotNil(t, providerMetadata)

// apps_get_logs_active_deployment_aggregate
_, ok := testPulumiPkg.Functions[packageName+":apps/v2:getAppsLogsActiveDeploymentAggregate"]
assert.True(t, ok, "Expected to find function getAppsLogsActiveDeploymentAggregate in the Pulumi package spec")

// apps_get_logs_aggregate
_, ok = testPulumiPkg.Functions[packageName+":apps/v2:getAppsLogsAggregate"]
assert.True(t, ok, "Expected to find function getAppsLogsAggregate in the Pulumi package spec")

// projects_list_resources
_, ok = testPulumiPkg.Functions[packageName+":projects/v2:listProjectsResources"]
assert.True(t, ok, "Expected to find function listProjectsResources in the Pulumi package spec")

// projects_list_resources_default
_, ok = testPulumiPkg.Functions[packageName+":projects/v2:listProjectsResourcesDefault"]
assert.True(t, ok, "Expected to find function listProjectsResourcesDefault in the Pulumi package spec")
}
30 changes: 30 additions & 0 deletions pkg/bug_105_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package pkg

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
)

// Test to ensure resources with discriminated request body types
// are generated with unique resource names if there is a collision.
func TestBug104(t *testing.T) {
mustReadTestOpenAPIDoc(t, filepath.Join("testdata", "bug_105_openapi.yml"))

openAPICtx := &OpenAPIContext{
Doc: *testOpenAPIDoc,
Pkg: &testPulumiPkg,
}

csharpNamespaces := map[string]string{
"": "Provider",
}

providerMetadata, _, err := openAPICtx.GatherResourcesFromAPI(csharpNamespaces)
assert.Nil(t, err)
assert.NotNil(t, providerMetadata)

_, ok := testPulumiPkg.Resources["fake-package:droplets/v2:DropletAction"]
assert.False(t, ok, "Resource DropletAction should not exist")
}
44 changes: 44 additions & 0 deletions pkg/multiple_paths_using_same_refs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package pkg

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
)

// TestMultiplePathsUsingSameRef tests that a type used in an allOf
// definition as well as a regular reference don't cause collisions.
// In other words, the random order in which request paths in an API
// spec are sometimes processed should not add/remove such a type
// based on which request is processed first.
func TestMultiplePathsUsingSameRef(t *testing.T) {
mustReadTestOpenAPIDoc(t, filepath.Join("testdata", "multiple_paths_using_same_refs.yml"))

openAPICtx := &OpenAPIContext{
Doc: *testOpenAPIDoc,
Pkg: &testPulumiPkg,
}

csharpNamespaces := map[string]string{
"": "Provider",
}

count := 0

// Due to the non-deterministic nature of iterating the request
// paths in the OpenAPI spec, we should execute this test a
// few times to guarantee we are safe from the issue we are
// testing for.
for {
if count >= 10 {
break
}
_, _, err := openAPICtx.GatherResourcesFromAPI(csharpNamespaces)
assert.Nil(t, err)

assert.Contains(t, testPulumiPkg.Types, "fake-package:resources:Meta")

count++
}
}
35 changes: 32 additions & 3 deletions pkg/naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,42 @@
package pkg

import (
"fmt"
"regexp"
"strings"
)

var numbersRegexp = regexp.MustCompile("[0-9]+[_]*[a-zA-Z]+")

// ToSdkName converts a property or attribute name to the lowerCamelCase convention that
// is used in Pulumi schema's properties.
func ToSdkName(s string) string {
if r := rune(s[0]); r >= 'A' && r <= 'Z' {
s = strings.ToLower(string(r)) + s[1:]
if startsWithNumber(s) {
return "_" + toCamelInitCase(s, false)
}
return s
return toCamelInitCase(s, false)
}

func startsWithNumber(s string) bool {
return numbersRegexp.Match([]byte(s))
}

// ToPascalCase converts a string to PascalCase.
func ToPascalCase(s string) string {
return toCamelInitCase(s, true)
}

// moduleToPascalCase converts a module name to PascalCase.
func moduleToPascalCase(mod string) string {
parts := strings.Split(mod, "/")

for i, p := range parts {
parts[i] = ToPascalCase(p)
}

return strings.Join(parts, "")
}

func toCamelInitCase(s string, initCase bool) string {
if s == strings.ToUpper(s) {
// lowercase the UPPER_SNAKE_CASE
Expand Down Expand Up @@ -51,3 +70,13 @@ func toCamelInitCase(s string, initCase bool) string {
}
return n
}

func addNameOverride(key, value string, m map[string]string) {
if v, ok := m[key]; ok && value != v {
panic(fmt.Errorf(
"mapping for %s already exists and has a value %s but a new mapping with value %s was requested",
key, v, value))
}

m[key] = value
}
19 changes: 19 additions & 0 deletions pkg/naming_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package pkg

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestToSdkName(t *testing.T) {
assert.Equal(t, "stringProp", ToSdkName("string_prop"))
assert.Equal(t, "stringProp", ToSdkName("string.prop"))
assert.Equal(t, "stringPropProp", ToSdkName("string.prop.prop"))
}

func TestStartsWithNumber(t *testing.T) {
assert.True(t, startsWithNumber("1_var"))
assert.True(t, startsWithNumber("1var"))
assert.False(t, startsWithNumber("var"))
}
Loading

0 comments on commit ae0c6a2

Please sign in to comment.