Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history 
Change-Id: I99f87a46cd96d96710aac4532a61361dcc7e2a27
  • Loading branch information
trollyxia committed Mar 22, 2024
1 parent 0fb3604 commit 9fabb83
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 159 deletions.
91 changes: 48 additions & 43 deletions bigtable/admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2190,26 +2190,30 @@ type AuthorizedViewConf struct {
TableID string
AuthorizedViewID string

AuthorizedViewTypeConf AuthorizedViewTypeConf
DeletionProtection DeletionProtection
// Types that are valid to be assigned to AuthorizedView:
// *SubsetView
AuthorizedView isAuthorizedView
DeletionProtection DeletionProtection
}

// AuthorizedViewTypeConf contains information about the type of an authorized view.
type AuthorizedViewTypeConf struct {
AuthorizedViewType AuthorizedViewType
// SubsetView is currently the only supported type as an AuthorizedView.
type SubsetView struct {
subsetView SubsetViewConf
}

SubsetView *SubsetViewConf
// A private interface that currently only implemented by SubsetView, ensuring that only SubsetView instances are accepted as an AuthorizedView.
// In the future if a new type of AuthorizedView is introduced, it should also implements this interface.
type isAuthorizedView interface {
isAuthorizedView()
}

// AuthorizedViewType represents the type of an authorized view.
type AuthorizedViewType int
func (*SubsetView) isAuthorizedView() {}

const (
// AuthorizedViewTypeUnspecified represents an unspecified authorized view type.
AuthorizedViewTypeUnspecified AuthorizedViewType = iota
// AuthorizedViewTypeSubsetView represents subset view type of an authorized view.
AuthorizedViewTypeSubsetView
)
// SubsetViewConf contains configuration specific to an authorized view of subset view type.
type SubsetViewConf struct {
RowPrefixes [][]byte
FamilySubsets map[string]FamilySubset
}

func (av AuthorizedViewConf) proto() *btapb.AuthorizedView {
var avp btapb.AuthorizedView
Expand All @@ -2223,30 +2227,31 @@ func (av AuthorizedViewConf) proto() *btapb.AuthorizedView {
break
}

switch avt := av.AuthorizedViewTypeConf.AuthorizedViewType; avt {
case AuthorizedViewTypeSubsetView:
switch avt := av.AuthorizedView.(type) {
case *SubsetView:
avp.AuthorizedView = &btapb.AuthorizedView_SubsetView_{
SubsetView: av.AuthorizedViewTypeConf.SubsetView.proto(),
SubsetView: avt.subsetView.proto(),
}
default:
break
}
return &avp
}

// SetSubsetView sets the AuthorizedViewType to AuthorizedViewTypeSubsetView and the subsetView pointer accordingly
func (av *AuthorizedViewTypeConf) SetSubsetView(s SubsetViewConf) error {
av.AuthorizedViewType = AuthorizedViewTypeSubsetView
av.SubsetView = &s
// SetSubsetView sets the AuthorizedView as SubsetView type with the specified config.
func (av *AuthorizedViewConf) SetSubsetView(s SubsetViewConf) error {
av.AuthorizedView = &SubsetView{subsetView: s}
return nil
}

// GetSubsetView returns an error if the type is not AuthorizedViewTypeSubsetView.
func (av *AuthorizedViewTypeConf) GetSubsetView() (*SubsetViewConf, error) {
if av.AuthorizedViewType != AuthorizedViewTypeSubsetView {
return nil, fmt.Errorf("not a subset view: :%v", av.AuthorizedViewType)
// GetSubsetView returns an error if the type of the AuthorizedView is not SubsetView.
func (av *AuthorizedViewConf) GetSubsetView() (*SubsetViewConf, error) {
switch avt := av.AuthorizedView.(type) {
case *SubsetView:
return &avt.subsetView, nil
default:
return nil, fmt.Errorf("not a subset view: %v", avt)
}
return av.SubsetView, nil
}

// FamilySubset represents a subset of a column family.
Expand All @@ -2255,12 +2260,6 @@ type FamilySubset struct {
QualifierPrefixes [][]byte
}

// SubsetViewConf contains configuration specific to an authorized view of subset view type.
type SubsetViewConf struct {
RowPrefixes [][]byte
FamilySubsets map[string]FamilySubset
}

// AddRowPrefix adds a new row prefix to the subset view.
func (s *SubsetViewConf) AddRowPrefix(prefix []byte) {
s.RowPrefixes = append(s.RowPrefixes, prefix)
Expand Down Expand Up @@ -2324,9 +2323,6 @@ func (ac *AdminClient) CreateAuthorizedView(ctx context.Context, conf *Authorize
if conf.TableID == "" || conf.AuthorizedViewID == "" {
return errors.New("both AuthorizedViewID and TableID are required")
}
if conf.AuthorizedViewTypeConf.AuthorizedViewType == AuthorizedViewTypeUnspecified {
return errors.New("AuthorizedViewType must be specified")
}

ctx = mergeOutgoingMetadata(ctx, ac.md)
req := &btapb.CreateAuthorizedViewRequest{
Expand All @@ -2345,7 +2341,6 @@ func (ac *AdminClient) GetAuthorizedView(ctx context.Context, tableID, authorize
Name: fmt.Sprintf("%s/tables/%s/authorizedViews/%s", ac.instancePrefix(), tableID, authorizedViewID),
}
var res *btapb.AuthorizedView
av := &AuthorizedViewConf{TableID: tableID, AuthorizedViewID: authorizedViewID}

err := gax.Invoke(ctx, func(ctx context.Context, _ gax.CallSettings) error {
var err error
Expand All @@ -2357,6 +2352,7 @@ func (ac *AdminClient) GetAuthorizedView(ctx context.Context, tableID, authorize
return nil, err
}

av := &AuthorizedViewConf{TableID: tableID, AuthorizedViewID: authorizedViewID}
if res.DeletionProtection {
av.DeletionProtection = Protected
} else {
Expand All @@ -2365,18 +2361,18 @@ func (ac *AdminClient) GetAuthorizedView(ctx context.Context, tableID, authorize
if res.GetSubsetView() != nil {
s := SubsetViewConf{}
s.fillConf(res.GetSubsetView())
av.AuthorizedViewTypeConf.SetSubsetView(s)
av.SetSubsetView(s)
}
return av, nil
}

// AuthorizedViews returns a list of the authorized views in the table. The names returned are fully
// qualified as projects/<project>/instances/<instance>/tables/<table>/authorizedViews/<authorizedView>
// AuthorizedViews returns a list of the authorized views in the table.
func (ac *AdminClient) AuthorizedViews(ctx context.Context, tableID string) ([]string, error) {
names := []string{}
prefix := fmt.Sprintf("%s/tables/%s", ac.instancePrefix(), tableID)

req := &btapb.ListAuthorizedViewsRequest{
Parent: fmt.Sprintf("%s/tables/%s", ac.instancePrefix(), tableID),
Parent: prefix,
View: btapb.AuthorizedView_NAME_ONLY,
}
var res *btapb.ListAuthorizedViewsResponse
Expand All @@ -2390,15 +2386,14 @@ func (ac *AdminClient) AuthorizedViews(ctx context.Context, tableID string) ([]s
}

for _, av := range res.AuthorizedViews {
names = append(names, av.Name)
names = append(names, strings.TrimPrefix(av.Name, prefix+"/authorizedViews/"))
}
return names, nil
}

// UpdateAuthorizedViewConf contains all the information necessary to update or partial update an authorized view.
type UpdateAuthorizedViewConf struct {
AuthorizedViewConf AuthorizedViewConf
UpdateMask []string
IgnoreWarnings bool
}

Expand All @@ -2410,9 +2405,19 @@ func (ac *AdminClient) UpdateAuthorizedView(ctx context.Context, conf UpdateAuth
}
av := conf.AuthorizedViewConf.proto()
av.Name = ac.authorizedViewPath(conf.AuthorizedViewConf.TableID, conf.AuthorizedViewConf.AuthorizedViewID)

updateMask := &field_mask.FieldMask{
Paths: []string{},
}
if conf.AuthorizedViewConf.DeletionProtection != None {
updateMask.Paths = append(updateMask.Paths, "deletion_protection")
}
if _, err := conf.AuthorizedViewConf.GetSubsetView(); err == nil {
updateMask.Paths = append(updateMask.Paths, "subset_view")
}
req := &btapb.UpdateAuthorizedViewRequest{
AuthorizedView: av,
UpdateMask: &field_mask.FieldMask{Paths: conf.UpdateMask},
UpdateMask: updateMask,
IgnoreWarnings: conf.IgnoreWarnings,
}
lro, err := ac.tClient.UpdateAuthorizedView(ctx, req)
Expand Down
45 changes: 32 additions & 13 deletions bigtable/admin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,9 @@ func TestTableAdmin_CreateAuthorizedView_DeletionProtection_Protected(t *testing

deletionProtection := Protected
err = c.CreateAuthorizedView(context.Background(), &AuthorizedViewConf{
TableID: "my-cool-table",
AuthorizedViewID: "my-cool-authorized-view",
AuthorizedViewTypeConf: AuthorizedViewTypeConf{
AuthorizedViewType: AuthorizedViewTypeSubsetView,
SubsetView: &SubsetViewConf{},
},
TableID: "my-cool-table",
AuthorizedViewID: "my-cool-authorized-view",
AuthorizedView: &SubsetView{subsetView: SubsetViewConf{}},
DeletionProtection: deletionProtection,
})
if err != nil {
Expand All @@ -346,12 +343,9 @@ func TestTableAdmin_CreateAuthorizedView_DeletionProtection_Unprotected(t *testi

deletionProtection := Unprotected
err := c.CreateAuthorizedView(context.Background(), &AuthorizedViewConf{
TableID: "my-cool-table",
AuthorizedViewID: "my-cool-authorized-view",
AuthorizedViewTypeConf: AuthorizedViewTypeConf{
AuthorizedViewType: AuthorizedViewTypeSubsetView,
SubsetView: &SubsetViewConf{},
},
TableID: "my-cool-table",
AuthorizedViewID: "my-cool-authorized-view",
AuthorizedView: &SubsetView{subsetView: SubsetViewConf{}},
DeletionProtection: deletionProtection,
})
if err != nil {
Expand Down Expand Up @@ -381,7 +375,6 @@ func TestTableAdmin_UpdateAuthorizedViewWithDeletionProtection(t *testing.T) {
AuthorizedViewID: "my-cool-authorized-view",
DeletionProtection: deletionProtection,
},
UpdateMask: []string{"deletion_protection"},
})
if err != nil {
t.Fatalf("UpdateAuthorizedView failed: %v", err)
Expand All @@ -401,6 +394,32 @@ func TestTableAdmin_UpdateAuthorizedViewWithDeletionProtection(t *testing.T) {
}
}

func TestTableAdmin_UpdateAuthorizedViewWithSubsetView(t *testing.T) {
mock := &mockTableAdminClock{}
c := setupTableClient(t, mock)

err := c.UpdateAuthorizedView(context.Background(), UpdateAuthorizedViewConf{
AuthorizedViewConf: AuthorizedViewConf{
TableID: "my-cool-table",
AuthorizedViewID: "my-cool-authorized-view",
AuthorizedView: &SubsetView{subsetView: SubsetViewConf{}},
},
})
if err != nil {
t.Fatalf("UpdateAuthorizedView failed: %v", err)
}
updateAuthorizedViewReq := mock.updateAuthorizedViewReq
if !cmp.Equal(updateAuthorizedViewReq.AuthorizedView.Name, "projects/my-cool-project/instances/my-cool-instance/tables/my-cool-table/authorizedViews/my-cool-authorized-view") {
t.Errorf("UpdateAuthorizedViewRequest does not match: AuthorizedViewName: %v, expected %v", updateAuthorizedViewReq.AuthorizedView.Name, "projects/my-cool-project/instances/my-cool-instance/tables/my-cool-table/authorizedViews/my-cool-authorized-view")
}
if !cmp.Equal(len(updateAuthorizedViewReq.UpdateMask.Paths), 1) {
t.Errorf("UpdateAuthorizedViewRequest does not match: UpdateMask has length of %d, expected %v", len(updateAuthorizedViewReq.UpdateMask.Paths), 1)
}
if !cmp.Equal(updateAuthorizedViewReq.UpdateMask.Paths[0], "subset_view") {
t.Errorf("UpdateAuthorizedViewRequest does not match: updateAuthorizedViewReq.UpdateMask.Paths[0]: %v, expected: %v", updateAuthorizedViewReq.UpdateMask.Paths[0], "subset_view")
}
}

type mockAdminClock struct {
btapb.BigtableInstanceAdminClient

Expand Down
Loading

0 comments on commit 9fabb83

Please sign in to comment.