Skip to content

Commit

Permalink
fix(update/etag): assign etag field on resource if request dont have
Browse files Browse the repository at this point in the history
Before tests were only generated if the update request object included
a etag field.

AIP specifies that update method can piggyback on the etag field on
the resource instead of adding it to the request object.

https://google.aip.dev/154#etags-on-request-methods
  • Loading branch information
thall committed Oct 31, 2024
1 parent a92fc78 commit c205cb0
Show file tree
Hide file tree
Showing 15 changed files with 669 additions and 6 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ Sample skips:
| update time | Field update_time should be updated when the resource is updated. | Generated only if all are true: <ul><li>has Create method</li><li>Create method does not return long-running operation</li><li>has Update method</li><li>Update method does not return long-running operation</li><li>has field 'update_time'</li></ul> |
| persisted | The updated resource should be persisted and reachable with Get. | Generated only if all are true: <ul><li>has Update method</li><li>Update method does not return long-running operation</li><li>has Get method</li></ul> |
| preserve create_time | The field create_time should be preserved when a '\*'-update mask is used. | Generated only if all are true: <ul><li>has Update method</li><li>Update method does not return long-running operation</li><li>has field 'create_time'</li><li>resource has any required fields</li></ul> |
| etag mismatch | Method should fail with Aborted if the supplied etag doesnt match the current etag value. | Generated only if all are true: <ul><li>has Update method</li><li>request has etag field</li><li>has field 'etag'</li></ul> |
| etag updated | Field etag should have a new value when the resource is successfully updated. | Generated only if all are true: <ul><li>has Update method</li><li>request has etag field</li><li>has field 'etag'</li></ul> |
| etag mismatch | Method should fail with Aborted if the supplied etag doesnt match the current etag value. | Generated only if all are true: <ul><li>has Update method</li><li>has field 'etag'</li></ul> |
| etag updated | Field etag should have a new value when the resource is successfully updated. | Generated only if all are true: <ul><li>has Update method</li><li>has field 'etag'</li></ul> |
| not found | Method should fail with NotFound if the resource does not exist. | Generated only if all are true: <ul><li>has Update method</li></ul> |
| invalid update mask | The method should fail with InvalidArgument if the update_mask is invalid. | Generated only if all are true: <ul><li>has Update method</li><li>Update method has update_mask</li></ul> |
| required fields | Method should fail with InvalidArgument if any required field is missing when called with '\*' update_mask. | Generated only if all are true: <ul><li>has Update method</li><li>resource has any required fields</li></ul> |
Expand Down
15 changes: 11 additions & 4 deletions internal/aiptest/update/etag.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ var etagMismatch = suite.Test{
},
OnlyIf: suite.OnlyIfs(
onlyif.HasMethod(aipreflect.MethodTypeUpdate),
onlyif.HasRequestEtag(aipreflect.MethodTypeUpdate),
onlyif.HasField("etag"),
),
Generate: func(f *protogen.GeneratedFile, scope suite.Scope) error {
Expand All @@ -32,8 +31,10 @@ var etagMismatch = suite.Test{
util.MethodUpdate{
Resource: scope.Resource,
Method: updateMethod,
Msg: "created",
Parent: "parent",
Name: "created.Name",
Etag: util.EtagLiteral("99999"),
EtagTest: true,
}.Generate(f, "_", "err", ":=")
f.P(ident.AssertEqual, "(t, ", ident.Codes(codes.Aborted), ",", ident.StatusCode, "(err), err)")
return nil
Expand All @@ -48,7 +49,6 @@ var etagUpdated = suite.Test{
},
OnlyIf: suite.OnlyIfs(
onlyif.HasMethod(aipreflect.MethodTypeUpdate),
onlyif.HasRequestEtag(aipreflect.MethodTypeUpdate),
onlyif.HasField("etag"),
),
Generate: func(f *protogen.GeneratedFile, scope suite.Scope) error {
Expand All @@ -65,9 +65,16 @@ var etagUpdated = suite.Test{
Parent: "parent",
Name: "created.Name",
Etag: "created.Etag",
EtagTest: true,
}.Generate(f, "updated", "err", ":=")
f.P(ident.AssertNilError, "(t, err)")
f.P(ident.AssertCheck, "(t, updated.Etag != created.Etag)")

if !util.ReturnsLRO(updateMethod.Desc) {
// only assert etag is different if the resource is returned.
f.P(ident.AssertCheck, "(t, updated.Etag != created.Etag)")
} else {
f.P("_ = updated") // prevent unused error.
}
return nil
},
}
9 changes: 9 additions & 0 deletions internal/util/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ type MethodUpdate struct {
Msg string
UpdateMask []string
Etag string
EtagTest bool
}

func (m MethodUpdate) Generate(f *protogen.GeneratedFile, response, err, assign string) {
Expand All @@ -111,6 +112,14 @@ func (m MethodUpdate) Generate(f *protogen.GeneratedFile, response, err, assign
}
f.P("msg.Name = ", m.Name)
}
if m.EtagTest && !HasEtagField(m.Method.Input.Desc) && HasEtagField(m.Method.Output.Desc) {
// Request object does not have an etag field, but the resource has.
if m.Etag != "" {
f.P("msg.Etag = ", m.Etag)
} else {
f.P(`msg.Etag = created.Etag // assign etag from the created resource`)
}
}
f.P(response, ", ", err, " ", assign, " fx.Service().", m.Method.GoName, "(fx.Context(), &", m.Method.Input.GoIdent, "{") //nolint:lll
if m.Msg != "" {
f.P(upper, ":", m.Msg, ",")
Expand Down
56 changes: 56 additions & 0 deletions proto/gen/einride/example/freight/v1/freight_service_aiptest.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit c205cb0

Please sign in to comment.