DefaultCheck should apply default values (#239)

Currently, defaults get applied by non-standard `Check` but not by
`infer.DefaultCheck`. Since the purpose of `infer.DefaultCheck` is to
mirror the default behavior of the Check call, defaults should be
applied.

Resolves #238

infer/resource.go

@@ -851,57 +851,75 @@ func (*derivedResourceController[R, I, O]) getInstance() *R {
 }
 
 func (rc *derivedResourceController[R, I, O]) Check(ctx context.Context, req p.CheckRequest) (p.CheckResponse, error) {
+	encoder, i, failures, err := decodeCheckingMapErrors[I](req.News)
+	if err != nil {
+		return p.CheckResponse{}, err
+	}
+	if len(failures) > 0 {
+		return p.CheckResponse{
+			Inputs:   req.News,
+			Failures: failures,
+		}, nil
+	}
+
 	var r R
-	encoder, i, err := ende.Decode[I](req.News)
 	if r, ok := ((interface{})(r)).(CustomCheck[I]); ok {
 		// The user implemented check manually, so call that
 		i, failures, err := r.Check(ctx, req.Urn.Name(), req.Olds, req.News)
 		if err != nil {
 			return p.CheckResponse{}, err
 		}
 		inputs, err := encoder.Encode(i)
-		if err != nil {
-			return p.CheckResponse{}, err
-		}
 		return p.CheckResponse{
 			Inputs:   inputs,
 			Failures: failures,
-		}, nil
-	}
-	if err == nil {
-		if err := applyDefaults(&i); err != nil {
-			return p.CheckResponse{}, fmt.Errorf("unable to apply defaults: %w", err)
-		}
-
-		inputs, err := encoder.Encode(i)
-
-		return p.CheckResponse{
-			Inputs: inputs,
 		}, err
 	}
 
-	failures, e := checkFailureFromMapError(err)
-	if e != nil {
-		return p.CheckResponse{}, e
+	if i, err = defaultCheck(i); err != nil {
+		return p.CheckResponse{}, fmt.Errorf("unable to apply defaults: %w", err)
 	}
 
+	inputs, err := encoder.Encode(i)
+
 	return p.CheckResponse{
-		Inputs:   req.News,
-		Failures: failures,
-	}, nil
+		Inputs: inputs,
+	}, err
 }
 
-// Ensure that `inputs` can deserialize cleanly into `I`.
+// DefaultCheck verifies that `inputs` can deserialize cleanly into `I`. This is the default
+// validation that is performed when leaving `Check` unimplemented.
+// It also adds defaults to `inputs` as necessary, as defined by `Annotator.SetDefault“.
 func DefaultCheck[I any](inputs resource.PropertyMap) (I, []p.CheckFailure, error) {
-	_, i, err := ende.Decode[I](inputs)
-	if err == nil {
-		return i, nil, nil
+	_, i, failures, err := decodeCheckingMapErrors[I](inputs)
+	if err != nil || len(failures) > 0 {
+		return i, failures, err
+	}
+
+	i, err = defaultCheck(i)
+	return i, nil, err
+}
+
+func defaultCheck[I any](i I) (I, error) {
+	if err := applyDefaults(&i); err != nil {
+		return i, fmt.Errorf("unable to apply defaults: %w", err)
+	}
+	return i, nil
+}
+
+func decodeCheckingMapErrors[I any](inputs resource.PropertyMap) (ende.Encoder, I, []p.CheckFailure, error) {
+	encoder, i, err := ende.Decode[I](inputs)
+	if err != nil {
+		failures, e := checkFailureFromMapError(err)
+		return encoder, i, failures, e
 	}
 
-	failures, e := checkFailureFromMapError(err)
-	return i, failures, e
+	return encoder, i, nil, nil
 }
 
+// err is nil -> nil, nil
+// all err.Failures are FieldErrors -> []p.CheckFailure, nil
+// otherwise -> unspecified, err
 func checkFailureFromMapError(err mapper.MappingError) ([]p.CheckFailure, error) {
 	if err == nil {
 		return nil, nil

infer/resource_test.go

@@ -449,3 +449,58 @@ func TestHydrateFromState(t *testing.T) {
 		}),
 	))
 }
+
+type checkResource struct {
+	P1 string `pulumi:"str,optional"`
+}
+
+const defaultValue = "default"
+
+func (c *checkResource) Annotate(a Annotator) {
+	a.SetDefault(&c.P1, defaultValue)
+}
+
+type checkResourceOutput struct{}
+
+func (c checkResource) Create(context.Context, string, checkResource, bool,
+) (id string, output checkResourceOutput, err error) {
+	return "", checkResourceOutput{}, nil
+}
+
+func TestCheck(t *testing.T) {
+	t.Parallel()
+
+	for tcName, tc := range map[string]struct {
+		input    r.PropertyMap
+		expected string
+	}{
+		"applies default for missing value":     {nil, defaultValue},
+		"applies default for empty value":       {r.PropertyMap{"str": r.NewStringProperty("")}, defaultValue},
+		"no change when default is already set": {r.PropertyMap{"str": r.NewStringProperty(defaultValue)}, defaultValue},
+		"respects non-default value":            {r.PropertyMap{"str": r.NewStringProperty("different")}, "different"},
+	} {
+		tc := tc
+
+		t.Run("Check "+tcName, func(t *testing.T) {
+			t.Parallel()
+			res := Resource[checkResource]()
+			checkResp, err := res.Check(context.Background(), p.CheckRequest{
+				Urn:  "a:b:c",
+				Olds: r.PropertyMap{},
+				News: tc.input,
+			})
+			require.NoError(t, err)
+			assert.Empty(t, checkResp.Failures)
+			assert.True(t, checkResp.Inputs.HasValue("str"))
+			assert.Equal(t, tc.expected, checkResp.Inputs["str"].StringValue())
+		})
+
+		t.Run("DefaultCheck "+tcName, func(t *testing.T) {
+			t.Parallel()
+			in, failures, err := DefaultCheck[checkResource](tc.input)
+			require.NoError(t, err)
+			assert.Empty(t, failures)
+			assert.Equal(t, tc.expected, in.P1)
+		})
+	}
+}