Skip to content

Commit

Permalink
chore(internal/gengapic): refactor checkIAMPolicyOverrides (#1530)
Browse files Browse the repository at this point in the history
At the moment checkIAMPolicyOverrides both checks if IAMPolicyOverrides
is needed and sets the value of g.hasIAMPolicyOverrides which is
unexpected.

Rename checkIAMPolicyOverrides to containsIAMPolicyOverrides, and return
a boolean value to be set outside the function.
  • Loading branch information
julieqiu authored May 30, 2024
1 parent 0aed80c commit 0fbdb35
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 15 deletions.
4 changes: 3 additions & 1 deletion internal/gengapic/gengapic.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ func Gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse
return &g.resp, nil
}

g.checkIAMPolicyOverrides(genServs)
if g.containsIAMPolicyOverrides(genServs) {
g.hasIAMPolicyOverrides = true
}

if g.serviceConfig != nil {
g.apiName = g.serviceConfig.GetTitle()
Expand Down
10 changes: 5 additions & 5 deletions internal/gengapic/mixins.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,24 +206,24 @@ func (g *generator) hasLocationMixin() bool {
return len(g.mixins["google.cloud.location.Locations"]) > 0 && len(g.serviceConfig.GetApis()) > 1
}

// checkIAMPolicyOverrides determines if any of the given services define an
// containsIAMPolicyOverrides determines if any of the given services define an
// IAMPolicy RPC and sets the hasIAMpolicyOverrides generator flag if so. If set
// to true, the IAMPolicy mixin will not be generated on any service client. This
// is for backwards compatibility with existing IAMPolicy redefinitions.
func (g *generator) checkIAMPolicyOverrides(servs []*descriptorpb.ServiceDescriptorProto) {
func (g *generator) containsIAMPolicyOverrides(servs []*descriptorpb.ServiceDescriptorProto) bool {
iam, hasMixin := g.mixins["google.iam.v1.IAMPolicy"]
if !hasMixin {
return
return false
}

for _, s := range servs {
for _, iamMethod := range iam {
if hasMethod(s, iamMethod.GetName()) {
g.hasIAMPolicyOverrides = true
return
return true
}
}
}
return false
}

// includeMixinInputFile determines if the given proto file name matches
Expand Down
14 changes: 5 additions & 9 deletions internal/gengapic/mixins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func TestHasIAMPolicyMixin(t *testing.T) {
}
}

func TestCheckIAMPolicyOverrides(t *testing.T) {
func TestContainsIAMPolicyOverrides(t *testing.T) {
g := &generator{
mixins: make(mixins),
}
Expand All @@ -181,18 +181,14 @@ func TestCheckIAMPolicyOverrides(t *testing.T) {
},
}
servs := []*descriptorpb.ServiceDescriptorProto{serv, other}
var want bool
g.checkIAMPolicyOverrides(servs)
if got := g.hasIAMPolicyOverrides; !cmp.Equal(got, want) {
t.Errorf("TestCheckIAMPolicyOverrides wanted %v but got %v", want, got)
if g.containsIAMPolicyOverrides(servs) {
t.Errorf("TestContainsIAMPolicyOverrides = true; want = false")
}

want = true
g.mixins["google.iam.v1.IAMPolicy"] = iamPolicyMethods()
serv.Method = append(serv.Method, &descriptorpb.MethodDescriptorProto{Name: proto.String("GetIamPolicy")})
g.checkIAMPolicyOverrides(servs)
if got := g.hasIAMPolicyOverrides; !cmp.Equal(got, want) {
t.Errorf("TestCheckIAMPolicyOverrides wanted %v but got %v", want, got)
if !g.containsIAMPolicyOverrides(servs) {
t.Errorf("TestContainsIAMPolicyOverrides = false; want = true")
}
}

Expand Down

0 comments on commit 0fbdb35

Please sign in to comment.