Skip to content

Commit

Permalink
Merge #11701
Browse files Browse the repository at this point in the history
11701: Revert "Simplified invokes: SDK-gen and program-gen implementation for dotnet and nodejs" r=iwahbe a=AaronFriel

Reverts #11418.

Fixes #11699. The addition of the `ReturnType` field on `schema.FunctionSpec` broke the API contract between Pulumi's codegen package and tfbridge and other tools.

From my comment on #11699.

> I was able to bisect pu/pu to a commit that had the issue, but couldn't reproduce the behavior with a synthetic schema. PR #11418 should be reverted because it introduces the breaking change, and before that PR is reintroduced, the breaking change to the schema should be addressed.
> 
> Notes on root causing:
> 
> Adding an unserialized `ReturnType` field on the `schema.FunctionSpec` type was a breaking change in the API contract with the tfgen bridge and any other tools that generate a PackageSpec directly (without marshaling/unmarshaling) before binding it and calling `GeneratePackage`.
> 
> In [cd6f658](cd6f658) we can see that a regression test fails to catch this case, the generated code is correct and matches the "before" case! That's because the regression tests in Pulumi load schemas from JSON, so the `ReturnType` field is populated. That occurs in the implementation of `unmarshalFunctionSpec` at line 1656:
> 
> https://github.com/pulumi/pulumi/blob/78c9bf4ee4def5130bfd5b29346ece7c61cec08a/pkg/codegen/schema/schema.go#L1649-L1658
> 
> This function in the bridge is unaware of the new field, and so it only populates `Outputs`.
> 
> https://github.com/pulumi/pulumi-terraform-bridge/blob/36b10921bfd25a5cabf0a342d0e449278a256a8b/pkg/tfgen/generate_schema.go#L607-L638




Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
  • Loading branch information
bors[bot] and AaronFriel committed Dec 21, 2022
2 parents 0f23e35 + a682a1a commit 6f35716
Show file tree
Hide file tree
Showing 90 changed files with 287 additions and 6,077 deletions.
9 changes: 3 additions & 6 deletions pkg/codegen/docs/gen.go
Expand Up @@ -1721,14 +1721,11 @@ func (mod *modContext) getTypes(member interface{}, types nestedTypeUsageInfo) {
mod.getTypes(m.Function, types)
}
case *schema.Function:
if t.Inputs != nil && !t.MultiArgumentInputs {
if t.Inputs != nil {
mod.getNestedTypes(t.Inputs, types, true)
}

if t.ReturnType != nil {
if objectType, ok := t.ReturnType.(*schema.ObjectType); ok && objectType != nil {
mod.getNestedTypes(objectType, types, false)
}
if t.Outputs != nil {
mod.getNestedTypes(t.Outputs, types, false)
}
}
}
Expand Down
7 changes: 2 additions & 5 deletions pkg/codegen/docs/gen_function.go
Expand Up @@ -443,11 +443,8 @@ func (mod *modContext) genFunction(f *schema.Function) functionDocArgs {
if f.Inputs != nil {
inputProps[lang] = mod.getProperties(f.Inputs.Properties, lang, true, false, false)
}
if f.ReturnType != nil {
if objectObject, ok := f.ReturnType.(*schema.ObjectType); ok {
outputProps[lang] = mod.getProperties(objectObject.Properties,
lang, false, false, false)
}
if f.Outputs != nil {
outputProps[lang] = mod.getProperties(f.Outputs.Properties, lang, false, false, false)
}
}

Expand Down
10 changes: 4 additions & 6 deletions pkg/codegen/docs/gen_method.go
Expand Up @@ -78,11 +78,9 @@ func (mod *modContext) genMethod(r *schema.Resource, m *schema.Method) methodDoc
inputProps[lang] = props
}
}
if f.ReturnType != nil {
if objectType, ok := f.ReturnType.(*schema.ObjectType); ok && objectType != nil {
outputProps[lang] = mod.getPropertiesWithIDPrefixAndExclude(objectType.Properties, lang, false, false, false,
fmt.Sprintf("%s_result_", m.Name), nil)
}
if f.Outputs != nil {
outputProps[lang] = mod.getPropertiesWithIDPrefixAndExclude(f.Outputs.Properties, lang, false, false, false,
fmt.Sprintf("%s_result_", m.Name), nil)
}
}

Expand Down Expand Up @@ -326,7 +324,7 @@ func (mod *modContext) getMethodResult(r *schema.Resource, m *schema.Method) map

var resultTypeName string
for _, lang := range dctx.supportedLanguages {
if m.Function.ReturnType != nil {
if m.Function.Outputs != nil && len(m.Function.Outputs.Properties) > 0 {
def, err := mod.pkg.Definition()
contract.AssertNoError(err)
resultTypeName = dctx.getLanguageDocHelper(lang).GetMethodResultName(def, mod.mod, r, m)
Expand Down
20 changes: 2 additions & 18 deletions pkg/codegen/dotnet/doc.go
Expand Up @@ -110,24 +110,8 @@ func (d DocLanguageHelper) GetMethodName(m *schema.Method) string {
func (d DocLanguageHelper) GetMethodResultName(pkg *schema.Package, modName string, r *schema.Resource,
m *schema.Method) string {

var returnType *schema.ObjectType
if m.Function.ReturnType != nil {
if objectType, ok := m.Function.ReturnType.(*schema.ObjectType); ok {
returnType = objectType
} else {
typeDetails := map[*schema.ObjectType]*typeDetails{}
mod := &modContext{
pkg: pkg.Reference(),
mod: modName,
typeDetails: typeDetails,
namespaces: d.Namespaces,
}
return mod.typeString(m.Function.ReturnType, "", false, false, false)
}
}

if info, ok := pkg.Language["csharp"].(CSharpPackageInfo); ok {
if info.LiftSingleValueMethodReturns && returnType != nil && len(returnType.Properties) == 1 {
if info.LiftSingleValueMethodReturns && m.Function.Outputs != nil && len(m.Function.Outputs.Properties) == 1 {
typeDetails := map[*schema.ObjectType]*typeDetails{}
mod := &modContext{
pkg: pkg.Reference(),
Expand All @@ -136,7 +120,7 @@ func (d DocLanguageHelper) GetMethodResultName(pkg *schema.Package, modName stri
namespaces: d.Namespaces,
rootNamespace: info.GetRootNamespace(),
}
return mod.typeString(returnType.Properties[0].Type, "", false, false, false)
return mod.typeString(m.Function.Outputs.Properties[0].Type, "", false, false, false)
}
}
return fmt.Sprintf("%s.%sResult", resourceName(r), d.GetMethodName(m))
Expand Down

0 comments on commit 6f35716

Please sign in to comment.