chore(docs): policy service contribution docs (#1842)

### Proposed Changes

* Adds a contributing guide to the various policy services linking to
existing documentation as appropriate or providing current conventions

### 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

---------

Co-authored-by: Ryan Yanulites <[email protected]>

service/policy/CONTRIBUTING.md

@@ -0,0 +1,35 @@
+# Contributing to Policy
+
+Policy in OpenTDF comprises a set of CRUD services rolled up into a single "policy" service
+registered to the OpenTDF platform.
+
+Each Policy Object is linked relationally at the database level (currently PostgreSQL), and
+there are conventions to the services and codebase.
+
+## Database
+
+See the [database readme](./db/README.md) for context.
+
+## Protos
+
+New policy protos are expected to follow the following conventions (provided as a checklist for development
+convenience).
+
+- [ ] Fields are validated by in-proto validators as much as possible (reducing in-service validation logic).
+    - [ ] Unit tests are written for the validation (see [./attributes/attributes_test.go](./attributes/attributes_test.go))
+- [ ] Proto fields:
+    - [ ] order required fields first, optional after
+    - [ ] document required fields as `// Required` and optional as `// Optional`
+    - [ ] reserve a reasonable number of field indexes for required (i.e. 1-100 used for required, 100+ are used for optional)
+- [ ] Pagination follows conventions laid out [in the ADR](./adr/0002-pagination-list-rpcs.md)
+
+## Services
+
+- [ ] CRUD RPCs for Policy objects that retroactively affect access to existing TDFs are served by the [`unsafe` service](./unsafe/)
+- [ ] Audit records follow the conventions [in the ADR](./adr/0000-current-state.md)
+- [ ] CRUD RPCs that affect multiple objects employ transactions as documented [in the ADR](./adr/0003-database-transactions.md)
+- [ ] Any write RPCs either employ a transaction for a read after write scenario, or populate responses from initial request + RETURNING
+    clauses in SQL (proactively avoiding potential data consistency issues at future scale)
+- [ ] Pagination follows conventions laid out [in the ADR](./adr/0002-pagination-list-rpcs.md)
+
+

service/policy/adr/0003-database-transactions.md

@@ -3,11 +3,10 @@
 ## Table of Contents
 
 - [Background](#background)
-- [Chosen Option](#chosen-option)
-- [Considered Options](#considered-options)
-  - [LIMIT + OFFSET](#limit--offset)
-  - [Keyset Pagination](#keyset-pagination)
-  - [Cursor Pagination](#cursor-pagination)
+- [Use Cases](#use-cases)
+- [Platform DB Package Changes](#platform-db-package-changes)
+- [PolicyDBClient Changes](#policydbclient-changes)
+- [Usage Example](#usage-example)
 
 ## Background
 
@@ -22,60 +21,90 @@ Primary drivers for implementing database transactions:
 > [!NOTE]
 > There are likely to be more as the service matures and more complex operations are required.
 
-## Implementation
+## Platform DB Package Changes
 
-An abstraction over transactional operations will be introduced to hopefully simplify support for multiple database providers in the future:
+A `Begin` method will be added to the `PgxIface` interface to allow for the creation of transactions:
 
 ```go
-type DbTransaction interface {
-	Commit(ctx context.Context) error
-	Rollback(ctx context.Context)
+type PgxIface interface {
++	Begin(ctx context.Context) (pgx.Tx, error)
 }
 ```
 
 ### PolicyDBClient Changes
 
-New methods for creating transactions and executing operations will be added:
+A new `RunInTx` method for performing operations within a transaction will be added to the `PolicyDBClient` struct:
 
-```go
-// Starts a new transaction and returns it
-func (c *PolicyDBClient) Begin() (DbTransaction, error) {
-    // use desired database client to start a new transaction
-    // - initial implementation will use pgx.Tx with Postgres
-}
-```
 
 ```go
-// provides a PolicyDBClient instance using the transaction's connection
-func (c *PolicyDBClient) WithTx(tx DbTransaction) *PolicyDBClient {
-    // use desired database client to create a new PolicyDBClient instance with the transaction's connection
-    // - initial implementation will use pgx.Tx with Postgres
+// Creates a new transaction and provides the caller with a handler func to perform operations within the transaction.
+// If the handler func returns an error, the transaction will be rolled back.
+// If the handler func returns nil, the transaction will be committed. 
+func (c *PolicyDBClient) RunInTx(ctx context.Context, query func(txClient *PolicyDBClient) error) error {
+	tx, err := c.Client.Pgx.Begin(ctx)
+	if err != nil {
+		return fmt.Errorf("%w: %w", db.ErrTxBeginFailed, err)
+	}
+
+	txClient := &PolicyDBClient{c.Client, c.logger, c.Queries.WithTx(tx), c.listCfg}
+
+	err = query(txClient)
+	if err != nil {
+		c.logger.WarnContext(ctx, "error during DB transaction, rolling back")
+
+		if rollbackErr := tx.Rollback(ctx); rollbackErr != nil {
+			// this should never happen, but if it does, we want to know about it
+			return fmt.Errorf("%w, transaction [%w]: %w", db.ErrTxRollbackFailed, err, rollbackErr)
+		}
+
+		return err
+	}
+
+	if err = tx.Commit(ctx); err != nil {
+		return fmt.Errorf("%w: %w", db.ErrTxCommitFailed, err)
+	}
+
+	return nil
 }
 ```
 
 ### Usage Example
 
-It is recommended to create the transaction within the service layer, as calling `WithTx()` from the service layer will inherently provide the transaction to any further `PolicyDBClient` methods called within the DB layer.
+As a general rule, it is recommended to use the `RunInTx()` method within service layer methods to avoid confusion with transaction handling or duplicating a transaction across the service and database layers.  Any additional `PolicyDBClient` methods triggered from the original method call at the service layer, will inherit the transaction created by `RunInTx`.
 
 ```go
-tx, err := dbClient.Begin()
-if err != nil {
-    // handle error
-}
-defer tx.Rollback(ctx)
-
-result, err := dbClient.WithTx(tx).SomeOperation(ctx, ...params)
-if err != nil {
-    // handle error
-}
-
-nextResult, err := dbClient.WithTx(tx).AnotherOperation(ctx, ...params)
-if err != nil {
-    // handle error
-}
-
-err = tx.Commit(ctx)
-if err != nil {
-    // handle error
+func (s AttributesService) CreateAttribute(ctx context.Context,
+	req *connect.Request[attributes.CreateAttributeRequest],
+) (*connect.Response[attributes.CreateAttributeResponse], error) {
+	s.logger.Debug("creating new attribute definition", slog.String("name", req.Msg.GetName()))
+	rsp := &attributes.CreateAttributeResponse{}
+
+	auditParams := audit.PolicyEventParams{
+		ObjectType: audit.ObjectTypeAttributeDefinition,
+		ActionType: audit.ActionTypeCreate,
+	}
+
+    // RunInTX() used here to ensure atomic creation of attribute definition and all values
+	err := s.dbClient.RunInTx(ctx, func(txClient *policydb.PolicyDBClient) error {
+		item, err := txClient.CreateAttribute(ctx, req.Msg)
+		if err != nil {
+			s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
+			return err
+		}
+
+		s.logger.Debug("created new attribute definition", slog.String("name", req.Msg.GetName()))
+
+		auditParams.ObjectID = item.GetId()
+		auditParams.Original = item
+		s.logger.Audit.PolicyCRUDSuccess(ctx, auditParams)
+
+		rsp.Attribute = item
+		return nil
+	})
+	if err != nil {
+		return nil, db.StatusifyError(err, db.ErrTextCreationFailed, slog.String("attribute", req.Msg.String()))
+	}
+
+	return connect.NewResponse(rsp), nil
 }
 ```

service/policy/db/README.md

@@ -8,6 +8,23 @@ Migrations are configurable (see [service configuration readme](../../../docs/co
 Goose runs [the migrations](./migrations/) sequentially, and each migration should have an associated ERD in markdown as well if there have been
 changes to the table relations in the policy schema.
 
+Each migration is named `YYYYMMDD<number>_effect.sql` (i.e. `20240101000001_add_new_object.sql`) so that
+goose can order them appropriately.
+
+Each migration should also get a `.md` of the same name beside it with a description of the change to
+the schema and motivation behind it.
+
+As of the time of writing this documentation, there is a CLI command on the overall platform binary to
+migrate up and down for testing.
+
+
+Migration checklist:
+
+- [ ] tested migrating up and down thoroughly with CRUD before/after
+- [ ] migration file `.sql` named appropriately
+- [ ] migration file contains a `.md` associated with it
+- [ ] overall schema updated with `make policy-erd-gen`
+
 ### Queries
 
 Historically, queries have been written in Go with [squirrel](https://github.com/Masterminds/squirrel).