Skip to content

Commit

Permalink
[sdk-gen] Use specialized missing required property exception (#1228)
Browse files Browse the repository at this point in the history
# Description

This PR adds a new custom exception `MissingRequiredPropertyException`
to the main Java SDK. Then SDK-gen uses this type and throws it as an
error when an input property is required but isn't specified. This fixes
#1199 where previously the users would receive a `NullPointerException`
instead.

Due to using Go templates and conditionals as follows:
```
{{ if $setter.IsRequired }} 
...
{{ end }}
```
The generated code emits an empty line when `IsRequired` is `false`
which is why you will see the new line diffs in the commits of this PR

## Checklist

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have updated the
[CHANGELOG-PENDING](https://github.com/pulumi/pulumi/blob/master/CHANGELOG_PENDING.md)
file with my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Service,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Service API version
<!-- @pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
  • Loading branch information
Zaid-Ajaj authored Oct 13, 2023
2 parents 353d99a + 7d572fd commit fe67fea
Show file tree
Hide file tree
Showing 1,715 changed files with 17,593 additions and 2,716 deletions.
8 changes: 2 additions & 6 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
### Improvements

- Adds `MissingRequiredPropertyException` to the main java SDK to be used later in the generated provider SDKs.
- Generated SDKs use specialized `MissingRequiredPropertyException` rather than throwing `NullPointerException` when a required input property is missing.

### Bug Fixes

- Fixes `builder()` implementation for result types where the identifier of the local variable defined for the result type collides with one of the fields of the result type.
- Adds `options.encoding = "UTF-8"` to the `compileJava` options so that it can handle non-ASCII characters in the source code, especially in the documentation comments of fields.
- Fixes MockMonitor reporting DeletedWith wasn't supported
### Bug Fixes
36 changes: 29 additions & 7 deletions pkg/codegen/java/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ func (pt *plainType) genInputType(ctx *classFileContext) error {

// Determine property types
propTypes := make([]TypeShape, len(pt.properties))
anyPropertyRequired := false
for i, prop := range pt.properties {
requireInitializers := !pt.args || isInputType(prop.Type)

Expand All @@ -503,6 +504,14 @@ func (pt *plainType) genInputType(ctx *classFileContext) error {
false, // outer optional
false, // inputless overload
)

if prop.IsRequired() {
anyPropertyRequired = true
}
}

if anyPropertyRequired {
ctx.imports.Ref(names.PulumiMissingRequiredPropertyException)
}

w := ctx.writer
Expand Down Expand Up @@ -599,6 +608,15 @@ func (pt *plainType) genInputType(ctx *classFileContext) error {
for propIndex, prop := range pt.properties {
propType := propTypes[propIndex]
fieldName := names.Ident(pt.mod.propertyName(prop)).AsProperty().Field()

if prop.IsRequired() && prop.DefaultValue == nil && prop.ConstValue == nil {
fprintf(w, " if ($.%s == null) {\n", fieldName)
fprintf(w, " throw new %s(\"%s\", \"%s\");\n",
ctx.ref(names.PulumiMissingRequiredPropertyException), pt.name, fieldName)
fprintf(w, " }\n")
continue
}

propRef := fmt.Sprintf("$.%s", fieldName)
propInit, err := dg.defaultValueExpr(
fmt.Sprintf("property of class %s", pt.name),
Expand Down Expand Up @@ -704,6 +722,7 @@ func (pt *plainType) genOutputType(ctx *classFileContext) error {
fprintf(w, "@%s\n", ctx.ref(names.CustomType))
fprintf(w, "public final class %s {\n", pt.name)

anyPropertyRequired := false
// Generate each output field.
for _, prop := range props {
fieldName := names.Ident(pt.mod.propertyName(prop))
Expand All @@ -721,7 +740,15 @@ func (pt *plainType) genOutputType(ctx *classFileContext) error {
isGetter: true,
})
fprintf(w, " private %s %s;\n", fieldType.ToCode(ctx.imports), fieldName)
if prop.IsRequired() {
anyPropertyRequired = true
}
}

if anyPropertyRequired {
ctx.imports.Ref(names.PulumiMissingRequiredPropertyException)
}

if len(props) > 0 {
fprintf(w, "\n")
}
Expand Down Expand Up @@ -810,12 +837,6 @@ func (pt *plainType) genOutputType(ctx *classFileContext) error {
})

setterName := propertyName.AsProperty().Setter()
assignment := func(propertyName names.Ident) string {
if prop.IsRequired() {
return fmt.Sprintf("this.%s = %s.requireNonNull(%s)", propertyName, ctx.ref(names.Objects), propertyName)
}
return fmt.Sprintf("this.%s = %s", propertyName, propertyName)
}

// add setter
var setterAnnotation string
Expand All @@ -828,7 +849,8 @@ func (pt *plainType) genOutputType(ctx *classFileContext) error {
SetterName: setterName,
PropertyType: propertyType.ToCode(ctx.imports),
PropertyName: propertyName.String(),
Assignment: assignment(propertyName),
Assignment: fmt.Sprintf("this.%s = %s", propertyName, propertyName),
IsRequired: prop.IsRequired(),
ListType: propertyType.ListType(ctx),
Annotations: []string{setterAnnotation},
})
Expand Down
4 changes: 4 additions & 0 deletions pkg/codegen/java/names/known.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ var Optional = JavaUtil.Dot("Optional")

var Pulumi = Ident("com").FQN().Dot("pulumi")

var PulumiExceptions = Pulumi.Dot("exceptions")

var PulumiMissingRequiredPropertyException = PulumiExceptions.Dot("MissingRequiredPropertyException")

var PulumiCore = Pulumi.Dot("core")

var PulumiAsset = Pulumi.Dot("asset")
Expand Down
4 changes: 4 additions & 0 deletions pkg/codegen/java/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ type builderSetterTemplateContext struct {
PropertyType string
PropertyName string
Assignment string
IsRequired bool
ListType string
Annotations []string
}
Expand Down Expand Up @@ -173,6 +174,9 @@ const builderTemplateText = `{{ .Indent }}public static {{ .Name }} builder() {
{{ $.Indent }} {{ $annotation }}
{{- end }}
{{ $.Indent }} public {{ $.Name }} {{ $setter.SetterName }}({{ $setter.PropertyType }} {{ $setter.PropertyName }}) {
{{ if $setter.IsRequired }}{{ $.Indent }} if ({{ $setter.PropertyName }} == null) {
{{ $.Indent }} throw new MissingRequiredPropertyException("{{ $.ResultType }}", "{{ $setter.PropertyName }}");
{{ $.Indent }} }{{ end }}
{{ $.Indent }} {{ $setter.Assignment }};
{{ $.Indent }} return this;
{{ $.Indent }} }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Integer;
import java.lang.String;
import java.util.List;
Expand Down Expand Up @@ -236,9 +237,15 @@ public Builder version(Integer version) {
}

public AppSecActivationsArgs build() {
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.notificationEmails = Objects.requireNonNull($.notificationEmails, "expected parameter 'notificationEmails' to be non-null");
$.version = Objects.requireNonNull($.version, "expected parameter 'version' to be non-null");
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecActivationsArgs", "configId");
}
if ($.notificationEmails == null) {
throw new MissingRequiredPropertyException("AppSecActivationsArgs", "notificationEmails");
}
if ($.version == null) {
throw new MissingRequiredPropertyException("AppSecActivationsArgs", "version");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Boolean;
import java.lang.Integer;
import java.lang.String;
Expand Down Expand Up @@ -152,8 +153,12 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecAdvancedSettingsEvasivePathMatchArgs build() {
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.enablePathMatch = Objects.requireNonNull($.enablePathMatch, "expected parameter 'enablePathMatch' to be non-null");
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsEvasivePathMatchArgs", "configId");
}
if ($.enablePathMatch == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsEvasivePathMatchArgs", "enablePathMatch");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Integer;
import java.lang.String;
import java.util.Objects;
Expand Down Expand Up @@ -151,8 +152,12 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecAdvancedSettingsLoggingArgs build() {
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.logging = Objects.requireNonNull($.logging, "expected parameter 'logging' to be non-null");
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsLoggingArgs", "configId");
}
if ($.logging == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsLoggingArgs", "logging");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Integer;
import java.lang.String;
import java.util.Objects;
Expand Down Expand Up @@ -151,8 +152,12 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecAdvancedSettingsPragmaHeaderArgs build() {
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.pragmaHeader = Objects.requireNonNull($.pragmaHeader, "expected parameter 'pragmaHeader' to be non-null");
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPragmaHeaderArgs", "configId");
}
if ($.pragmaHeader == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPragmaHeaderArgs", "pragmaHeader");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Boolean;
import java.lang.Integer;
import java.lang.String;
Expand Down Expand Up @@ -235,11 +236,21 @@ public Builder extensions(String... extensions) {
}

public AppSecAdvancedSettingsPrefetchArgs build() {
$.allExtensions = Objects.requireNonNull($.allExtensions, "expected parameter 'allExtensions' to be non-null");
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.enableAppLayer = Objects.requireNonNull($.enableAppLayer, "expected parameter 'enableAppLayer' to be non-null");
$.enableRateControls = Objects.requireNonNull($.enableRateControls, "expected parameter 'enableRateControls' to be non-null");
$.extensions = Objects.requireNonNull($.extensions, "expected parameter 'extensions' to be non-null");
if ($.allExtensions == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPrefetchArgs", "allExtensions");
}
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPrefetchArgs", "configId");
}
if ($.enableAppLayer == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPrefetchArgs", "enableAppLayer");
}
if ($.enableRateControls == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPrefetchArgs", "enableRateControls");
}
if ($.extensions == null) {
throw new MissingRequiredPropertyException("AppSecAdvancedSettingsPrefetchArgs", "extensions");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Boolean;
import java.lang.Integer;
import java.lang.String;
Expand Down Expand Up @@ -150,9 +151,15 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecApiConstraintsProtectionArgs build() {
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.enabled = Objects.requireNonNull($.enabled, "expected parameter 'enabled' to be non-null");
$.securityPolicyId = Objects.requireNonNull($.securityPolicyId, "expected parameter 'securityPolicyId' to be non-null");
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecApiConstraintsProtectionArgs", "configId");
}
if ($.enabled == null) {
throw new MissingRequiredPropertyException("AppSecApiConstraintsProtectionArgs", "enabled");
}
if ($.securityPolicyId == null) {
throw new MissingRequiredPropertyException("AppSecApiConstraintsProtectionArgs", "securityPolicyId");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Integer;
import java.lang.String;
import java.util.Objects;
Expand Down Expand Up @@ -188,9 +189,15 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecApiRequestConstraintsArgs build() {
$.action = Objects.requireNonNull($.action, "expected parameter 'action' to be non-null");
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.securityPolicyId = Objects.requireNonNull($.securityPolicyId, "expected parameter 'securityPolicyId' to be non-null");
if ($.action == null) {
throw new MissingRequiredPropertyException("AppSecApiRequestConstraintsArgs", "action");
}
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecApiRequestConstraintsArgs", "configId");
}
if ($.securityPolicyId == null) {
throw new MissingRequiredPropertyException("AppSecApiRequestConstraintsArgs", "securityPolicyId");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Integer;
import java.lang.String;
import java.util.Objects;
Expand Down Expand Up @@ -225,10 +226,18 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecAttackGroupArgs build() {
$.attackGroup = Objects.requireNonNull($.attackGroup, "expected parameter 'attackGroup' to be non-null");
$.attackGroupAction = Objects.requireNonNull($.attackGroupAction, "expected parameter 'attackGroupAction' to be non-null");
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.securityPolicyId = Objects.requireNonNull($.securityPolicyId, "expected parameter 'securityPolicyId' to be non-null");
if ($.attackGroup == null) {
throw new MissingRequiredPropertyException("AppSecAttackGroupArgs", "attackGroup");
}
if ($.attackGroupAction == null) {
throw new MissingRequiredPropertyException("AppSecAttackGroupArgs", "attackGroupAction");
}
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecAttackGroupArgs", "configId");
}
if ($.securityPolicyId == null) {
throw new MissingRequiredPropertyException("AppSecAttackGroupArgs", "securityPolicyId");
}
return $;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.pulumi.core.Output;
import com.pulumi.core.annotations.Import;
import com.pulumi.exceptions.MissingRequiredPropertyException;
import java.lang.Integer;
import java.lang.String;
import java.util.List;
Expand Down Expand Up @@ -160,9 +161,15 @@ public Builder securityPolicyId(String securityPolicyId) {
}

public AppSecByPassNetworkListArgs build() {
$.bypassNetworkLists = Objects.requireNonNull($.bypassNetworkLists, "expected parameter 'bypassNetworkLists' to be non-null");
$.configId = Objects.requireNonNull($.configId, "expected parameter 'configId' to be non-null");
$.securityPolicyId = Objects.requireNonNull($.securityPolicyId, "expected parameter 'securityPolicyId' to be non-null");
if ($.bypassNetworkLists == null) {
throw new MissingRequiredPropertyException("AppSecByPassNetworkListArgs", "bypassNetworkLists");
}
if ($.configId == null) {
throw new MissingRequiredPropertyException("AppSecByPassNetworkListArgs", "configId");
}
if ($.securityPolicyId == null) {
throw new MissingRequiredPropertyException("AppSecByPassNetworkListArgs", "securityPolicyId");
}
return $;
}
}
Expand Down
Loading

0 comments on commit fe67fea

Please sign in to comment.