Skip to content

Commit e34680e

Browse files
ryanulitryanulit
authored
feat(policy): sqlc queries refactor (#2541)
### Proposed Changes - `query.sql` file has been broken into separate files for each individual unit of work under a `/queries` folder - removed sqlc `*Queries` struct embedding from the `PolicyDBClient` and updated all sqlc gen'd queries to be lowercase so that they are not exposed to external packages - refactored queries used only by integration tests into `pgx.Exec` calls to keep them out of the `PolicyDBClient` functionality ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent 80c9c85 commit e34680e

37 files changed

+7922
-7951
lines changed

‎service/integration/attribute_fqns_test.go‎

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -255,12 +255,8 @@ func (s *AttributeFqnSuite) TestGetAttributeByFqn_WithAttributeDefKeysAssociated
255255
})
256256
s.Require().NoError(err)
257257

258-
// Remove the attribute
259-
_, err = s.db.PolicyClient.DeleteAttribute(s.ctx, attr.GetId())
260-
s.Require().NoError(err)
261-
262-
// Remove the namespace
263-
_, err = s.db.PolicyClient.DeleteNamespace(s.ctx, ns.GetId())
258+
// cascade delete will remove namespaces and all associated attributes and values
259+
_, err = s.db.PolicyClient.UnsafeDeleteNamespace(s.ctx, ns, ns.GetFqn())
264260
s.Require().NoError(err)
265261
}
266262

@@ -1676,28 +1672,28 @@ func (s *AttributeFqnSuite) Test_GrantsAreReturned() {
16761672
s.NotNil(kas)
16771673

16781674
// Create NS Grant
1679-
nsGrant, err := s.db.PolicyClient.AssignKeyAccessServerToNamespace(s.ctx, policydb.AssignKeyAccessServerToNamespaceParams{
1680-
NamespaceID: ns.GetId(),
1681-
KeyAccessServerID: kas.GetId(),
1682-
})
1675+
// use Pgx.Exec because INSERT is only for testing and should not be part of PolicyDBClient
1676+
nsGrant, err := s.db.PolicyClient.Pgx.Exec(s.ctx,
1677+
`INSERT INTO attribute_namespace_key_access_grants (namespace_id, key_access_server_id) VALUES ($1, $2)`,
1678+
ns.GetId(), kas.GetId())
16831679
s.Require().NoError(err)
1684-
s.NotNil(nsGrant)
1680+
s.NotNil(nsGrant.RowsAffected())
16851681

16861682
// Create Attribute Grant
1687-
attrGrant, err := s.db.PolicyClient.AssignKeyAccessServerToAttribute(s.ctx, policydb.AssignKeyAccessServerToAttributeParams{
1688-
AttributeDefinitionID: attr.GetId(),
1689-
KeyAccessServerID: kas.GetId(),
1690-
})
1683+
// use Pgx.Exec because INSERT is only for testing and should not be part of PolicyDBClient
1684+
attrGrant, err := s.db.PolicyClient.Pgx.Exec(s.ctx,
1685+
`INSERT INTO attribute_definition_key_access_grants (attribute_definition_id, key_access_server_id) VALUES ($1, $2)`,
1686+
attr.GetId(), kas.GetId())
16911687
s.Require().NoError(err)
1692-
s.NotNil(attrGrant)
1688+
s.NotNil(attrGrant.RowsAffected())
16931689

16941690
// Create Value Grant
1695-
valueGrant, err := s.db.PolicyClient.AssignKeyAccessServerToAttributeValue(s.ctx, policydb.AssignKeyAccessServerToAttributeValueParams{
1696-
AttributeValueID: attr.GetValues()[0].GetId(),
1697-
KeyAccessServerID: kas.GetId(),
1698-
})
1691+
// use Pgx.Exec because INSERT is only for testing and should not be part of PolicyDBClient
1692+
valueGrant, err := s.db.PolicyClient.Pgx.Exec(s.ctx,
1693+
`INSERT INTO attribute_value_key_access_grants (attribute_value_id, key_access_server_id) VALUES ($1, $2)`,
1694+
attr.GetValues()[0].GetId(), kas.GetId())
16991695
s.Require().NoError(err)
1700-
s.NotNil(valueGrant)
1696+
s.NotNil(valueGrant.RowsAffected())
17011697

17021698
// Get Namespace check for grant
17031699
nsGet, err := s.db.PolicyClient.GetNamespace(s.ctx, ns.GetId())

‎service/integration/kas_registry_key_test.go‎

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -400,14 +400,12 @@ func (s *KasRegistryKeySuite) Test_ListKeys_KasID_Limit_Success() {
400400
}
401401

402402
func (s *KasRegistryKeySuite) Test_RotateKey_Multiple_Attributes_Values_Namespaces_Success() {
403-
attrValueIDs := make([]string, 0)
404-
attrDefIDs := make([]string, 0)
405403
namespaceIDs := make([]string, 0)
406404
keyIDs := make([]string, 0)
407405
kasIDs := make([]string, 0)
408406

409407
defer func() {
410-
s.cleanupAttrs(attrValueIDs, namespaceIDs, attrDefIDs)
408+
s.cleanupNamespacesAndAttrsByIDs(namespaceIDs)
411409
s.cleanupKeys(keyIDs, kasIDs)
412410
}()
413411

@@ -426,8 +424,6 @@ func (s *KasRegistryKeySuite) Test_RotateKey_Multiple_Attributes_Values_Namespac
426424
namespaceMap := s.setupNamespaceForRotate(1, 1, keyMap[rotateKey].GetKey(), keyMap[nonRotateKey].GetKey())
427425
namespaceIDs = append(namespaceIDs, namespaceMap[rotateKey][0].GetId(), namespaceMap[nonRotateKey][0].GetId())
428426
attributeMap := s.setupAttributesForRotate(1, 1, 1, 1, namespaceMap, keyMap[rotateKey].GetKey(), keyMap[nonRotateKey].GetKey())
429-
attrDefIDs = append(attrDefIDs, attributeMap[rotateKey][0].GetId(), attributeMap[nonRotateKey][0].GetId())
430-
attrValueIDs = append(attrValueIDs, attributeMap[rotateKey][0].GetValues()[0].GetId(), attributeMap[nonRotateKey][0].GetValues()[0].GetId())
431427

432428
newKey := kasregistry.RotateKeyRequest_NewKey{
433429
KeyId: "new_key_id",
@@ -525,14 +521,12 @@ func (s *KasRegistryKeySuite) Test_RotateKey_Multiple_Attributes_Values_Namespac
525521
}
526522

527523
func (s *KasRegistryKeySuite) Test_RotateKey_Two_Attribute_Two_Namespace_0_AttributeValue_Success() {
528-
attrValueIDs := make([]string, 0)
529-
attrDefIDs := make([]string, 0)
530524
namespaceIDs := make([]string, 0)
531525
keyIDs := make([]string, 0)
532526
kasIDs := make([]string, 0)
533527

534528
defer func() {
535-
s.cleanupAttrs(attrValueIDs, namespaceIDs, attrDefIDs)
529+
s.cleanupNamespacesAndAttrsByIDs(namespaceIDs)
536530
s.cleanupKeys(keyIDs, kasIDs)
537531
}()
538532

@@ -551,7 +545,6 @@ func (s *KasRegistryKeySuite) Test_RotateKey_Two_Attribute_Two_Namespace_0_Attri
551545
namespaceMap := s.setupNamespaceForRotate(2, 2, keyMap[rotateKey].GetKey(), keyMap[nonRotateKey].GetKey())
552546
namespaceIDs = append(namespaceIDs, namespaceMap[rotateKey][0].GetId(), namespaceMap[rotateKey][1].GetId(), namespaceMap[nonRotateKey][0].GetId(), namespaceMap[nonRotateKey][1].GetId())
553547
attributeMap := s.setupAttributesForRotate(2, 2, 0, 0, namespaceMap, keyMap[rotateKey].GetKey(), keyMap[nonRotateKey].GetKey())
554-
attrDefIDs = append(attrDefIDs, attributeMap[rotateKey][0].GetId(), attributeMap[nonRotateKey][0].GetId(), attributeMap[rotateKey][1].GetId(), attributeMap[nonRotateKey][1].GetId())
555548

556549
newKey := kasregistry.RotateKeyRequest_NewKey{
557550
KeyId: "new_key_id",
@@ -1143,7 +1136,7 @@ func (s *KasRegistryKeySuite) Test_ListKeyMappings_ByID_OneAttrValue_Success() {
11431136
keyIDs = append(keyIDs, key.GetKey().GetId())
11441137
}
11451138
s.cleanupKeys(keyIDs, kasIDs)
1146-
s.deleteAttributes(namespaces, attributeDefs, attrValues)
1139+
s.cleanupNamespacesAndAttrs(namespaces)
11471140
}()
11481141
kasKey := s.createKeyAndKas()
11491142
kasKeys = append(kasKeys, kasKey)
@@ -1209,7 +1202,7 @@ func (s *KasRegistryKeySuite) Test_ListKeyMappings_By_Key_Success() {
12091202
keyIDs = append(keyIDs, key.GetKey().GetId())
12101203
}
12111204
s.cleanupKeys(keyIDs, kasIDs)
1212-
s.deleteAttributes(namespaces, attributeDefs, attrValues)
1205+
s.cleanupNamespacesAndAttrs(namespaces)
12131206
}()
12141207
kasKey := s.createKeyAndKas()
12151208
kasKeys = append(kasKeys, kasKey)
@@ -1308,7 +1301,7 @@ func (s *KasRegistryKeySuite) Test_ListKeyMappings_SameKeyId_DifferentKas_Succes
13081301
keyIDs = append(keyIDs, key.GetKey().GetId())
13091302
}
13101303
s.cleanupKeys(keyIDs, kasIDs)
1311-
s.deleteAttributes(namespaces, attributeDefs, attrValues)
1304+
s.cleanupNamespacesAndAttrs(namespaces)
13121305
}()
13131306

13141307
kasKey := s.createKeyAndKas()
@@ -1411,7 +1404,7 @@ func (s *KasRegistryKeySuite) Test_ListKeyMappings_Multiple_Keys_Pagination_Succ
14111404
keyIDs = append(keyIDs, key.GetKey().GetId())
14121405
}
14131406
s.cleanupKeys(keyIDs, kasIDs)
1414-
s.deleteAttributes(namespaces, attributeDefs, attrValues)
1407+
s.cleanupNamespacesAndAttrs(namespaces)
14151408
}()
14161409
for i := range 2 {
14171410
kasKey := s.createKeyAndKas()
@@ -1482,7 +1475,7 @@ func (s *KasRegistryKeySuite) Test_ListKeyMappings_Multiple_Mixed_Mappings() {
14821475
keyIDs = append(keyIDs, key.GetKey().GetId())
14831476
}
14841477
s.cleanupKeys(keyIDs, kasIDs)
1485-
s.deleteAttributes(namespaces, attributeDefs, attrValues)
1478+
s.cleanupNamespacesAndAttrs(namespaces)
14861479
}()
14871480

14881481
for range 3 {
@@ -1746,23 +1739,9 @@ func (s *KasRegistryKeySuite) setupAttributesForRotate(numAttrsToRotate, numAttr
17461739
}
17471740
}
17481741

1749-
func (s *KasRegistryKeySuite) cleanupAttrs(attrValueIDs []string, namespaceIDs []string, attributeIDs []string) {
1750-
for _, id := range attrValueIDs {
1751-
_, err := s.db.PolicyClient.DeleteAttributeValue(s.ctx, id)
1752-
s.Require().NoError(err)
1753-
}
1754-
for _, id := range namespaceIDs {
1755-
_, err := s.db.PolicyClient.DeleteNamespace(s.ctx, id)
1756-
s.Require().NoError(err)
1757-
}
1758-
for _, id := range attributeIDs {
1759-
_, err := s.db.PolicyClient.DeleteAttribute(s.ctx, id)
1760-
s.Require().NoError(err)
1761-
}
1762-
}
1763-
17641742
func (s *KasRegistryKeySuite) cleanupKeys(keyIDs []string, keyAccessServerIDs []string) {
1765-
err := s.db.PolicyClient.DeleteAllBaseKeys(s.ctx)
1743+
// use Pgx.Exec because DELETE is only for testing and should not be part of PolicyDBClient
1744+
_, err := s.db.PolicyClient.Pgx.Exec(s.ctx, "DELETE FROM base_keys")
17661745
s.Require().NoError(err)
17671746

17681747
for _, id := range keyIDs {
@@ -1846,17 +1825,22 @@ func validatePrivatePublicCtx(s *suite.Suite, expectedPrivCtx, expectedPubCtx []
18461825
})
18471826
}
18481827

1849-
func (s *KasRegistryKeySuite) deleteAttributes(namespaces []*policy.Namespace, attributeDefs []*policy.Attribute, attrValues []*policy.Value) {
1850-
for _, value := range attrValues {
1851-
_, err := s.db.PolicyClient.DeleteAttributeValue(s.ctx, value.GetId())
1852-
s.Require().NoError(err)
1853-
}
1854-
for _, def := range attributeDefs {
1855-
_, err := s.db.PolicyClient.DeleteAttribute(s.ctx, def.GetId())
1828+
// cascade delete will remove namespaces and all associated attributes and values
1829+
func (s *KasRegistryKeySuite) cleanupNamespacesAndAttrsByIDs(namespaceIDs []string) {
1830+
namespaces := make([]*policy.Namespace, len(namespaceIDs))
1831+
for i, id := range namespaceIDs {
1832+
ns, err := s.db.PolicyClient.GetNamespace(s.ctx, id)
18561833
s.Require().NoError(err)
1834+
s.NotNil(ns)
1835+
namespaces[i] = ns
18571836
}
1837+
s.cleanupNamespacesAndAttrs(namespaces)
1838+
}
1839+
1840+
// cascade delete will remove namespaces and all associated attributes and values
1841+
func (s *KasRegistryKeySuite) cleanupNamespacesAndAttrs(namespaces []*policy.Namespace) {
18581842
for _, ns := range namespaces {
1859-
_, err := s.db.PolicyClient.DeleteNamespace(s.ctx, ns.GetId())
1843+
_, err := s.db.PolicyClient.UnsafeDeleteNamespace(s.ctx, ns, ns.GetFqn())
18601844
s.Require().NoError(err)
18611845
}
18621846
}

‎service/integration/policy_test.go‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import (
66
"log/slog"
77
"testing"
88

9+
"github.com/opentdf/platform/protocol/go/policy"
10+
"github.com/opentdf/platform/protocol/go/policy/attributes"
11+
"github.com/opentdf/platform/protocol/go/policy/namespaces"
912
"github.com/opentdf/platform/service/internal/fixtures"
1013
"github.com/opentdf/platform/service/policy/db"
1114
"github.com/stretchr/testify/suite"
@@ -45,26 +48,29 @@ func (s *PolicyDBClientSuite) Test_RunInTx_CommitsOnSuccess() {
4548
)
4649

4750
txErr := s.db.PolicyClient.RunInTx(s.ctx, func(txClient *db.PolicyDBClient) error {
48-
nsID, err = txClient.Queries.CreateNamespace(s.ctx, db.CreateNamespaceParams{
51+
ns, err := txClient.CreateNamespace(s.ctx, &namespaces.CreateNamespaceRequest{
4952
Name: nsName,
5053
})
5154
s.Require().NoError(err)
52-
s.Require().NotNil(nsID)
55+
s.Require().NotNil(ns)
56+
nsID = ns.GetId()
5357

54-
attrID, err = txClient.Queries.CreateAttribute(s.ctx, db.CreateAttributeParams{
55-
NamespaceID: nsID,
58+
attr, err := txClient.CreateAttribute(s.ctx, &attributes.CreateAttributeRequest{
59+
NamespaceId: nsID,
5660
Name: attrName,
57-
Rule: db.AttributeDefinitionRuleALLOF,
61+
Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_ALL_OF,
5862
})
5963
s.Require().NoError(err)
60-
s.Require().NotNil(attrID)
64+
s.Require().NotNil(attr)
65+
attrID = attr.GetId()
6166

62-
valID, err = txClient.Queries.CreateAttributeValue(s.ctx, db.CreateAttributeValueParams{
63-
AttributeDefinitionID: attrID,
64-
Value: attrValue,
67+
val, err := txClient.CreateAttributeValue(s.ctx, attrID, &attributes.CreateAttributeValueRequest{
68+
AttributeId: attrID,
69+
Value: attrValue,
6570
})
6671
s.Require().NoError(err)
6772
s.Require().NotNil(valID)
73+
valID = val.GetId()
6874

6975
return nil
7076
})
@@ -94,19 +100,22 @@ func (s *PolicyDBClientSuite) Test_RunInTx_RollsBackOnFailure() {
94100
)
95101

96102
txErr := s.db.PolicyClient.RunInTx(s.ctx, func(txClient *db.PolicyDBClient) error {
97-
nsID, err = txClient.Queries.CreateNamespace(s.ctx, db.CreateNamespaceParams{
103+
ns, err := txClient.CreateNamespace(s.ctx, &namespaces.CreateNamespaceRequest{
98104
Name: nsName,
99105
})
100106
s.Require().NoError(err)
101-
s.Require().NotNil(nsID)
107+
s.Require().NotNil(ns)
108+
nsID = ns.GetId()
102109

103-
attrID, err = txClient.Queries.CreateAttribute(s.ctx, db.CreateAttributeParams{
104-
NamespaceID: "invalid_ns_id",
110+
attr, err := txClient.CreateAttribute(s.ctx, &attributes.CreateAttributeRequest{
111+
NamespaceId: "invalid_ns_id",
105112
Name: attrName,
106-
Rule: db.AttributeDefinitionRuleALLOF,
113+
Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_ALL_OF,
107114
})
108115
s.Require().Error(err)
109-
s.Require().Empty(attrID)
116+
s.Require().Empty(attr)
117+
attrID = attr.GetId()
118+
110119
return err
111120
})
112121
s.Require().Error(txErr)

‎service/policy/db/actions.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (c PolicyDBClient) GetAction(ctx context.Context, req *actions.GetActionReq
4848
return nil, db.ErrSelectIdentifierInvalid
4949
}
5050

51-
got, err := c.Queries.getAction(ctx, getActionParams)
51+
got, err := c.queries.getAction(ctx, getActionParams)
5252
if err != nil {
5353
return nil, db.WrapIfKnownInvalidQueryErr(err)
5454
}
@@ -73,7 +73,7 @@ func (c PolicyDBClient) ListActions(ctx context.Context, req *actions.ListAction
7373
return nil, db.ErrListLimitTooLarge
7474
}
7575

76-
list, err := c.Queries.listActions(ctx, listActionsParams{
76+
list, err := c.queries.listActions(ctx, listActionsParams{
7777
Limit: limit,
7878
Offset: offset,
7979
})
@@ -130,7 +130,7 @@ func (c PolicyDBClient) CreateAction(ctx context.Context, req *actions.CreateAct
130130
Metadata: metadataJSON,
131131
}
132132

133-
createdID, err := c.Queries.createCustomAction(ctx, createParams)
133+
createdID, err := c.queries.createCustomAction(ctx, createParams)
134134
if err != nil {
135135
return nil, db.WrapIfKnownInvalidQueryErr(err)
136136
}
@@ -166,7 +166,7 @@ func (c PolicyDBClient) UpdateAction(ctx context.Context, req *actions.UpdateAct
166166
Metadata: metadataJSON,
167167
}
168168

169-
count, err := c.Queries.updateCustomAction(ctx, updateParams)
169+
count, err := c.queries.updateCustomAction(ctx, updateParams)
170170
if err != nil {
171171
return nil, db.WrapIfKnownInvalidQueryErr(err)
172172
}
@@ -182,7 +182,7 @@ func (c PolicyDBClient) UpdateAction(ctx context.Context, req *actions.UpdateAct
182182
}
183183

184184
func (c PolicyDBClient) DeleteAction(ctx context.Context, req *actions.DeleteActionRequest) (*policy.Action, error) {
185-
count, err := c.Queries.deleteCustomAction(ctx, req.GetId())
185+
count, err := c.queries.deleteCustomAction(ctx, req.GetId())
186186
if err != nil {
187187
return nil, db.WrapIfKnownInvalidQueryErr(err)
188188
}

0 commit comments

Comments
 (0)