Addresses Review Comments, allRows => mapRows

RMI-PACTA · Sep 11, 2023 · c797f4a · c797f4a
1 parent 2faf307
commit c797f4a
Show file tree

Hide file tree

Showing 25 changed files with 125 additions and 347 deletions.
diff --git a/cmd/server/pactasrv/BUILD.bazel b/cmd/server/pactasrv/BUILD.bazel
@@ -13,7 +13,6 @@ go_library(
         "//db",
         "//openapi:pacta_generated",
         "//pacta",
-        "@com_github_silicon_ally_gqlerr//:gqlerr",
         "@org_uber_go_zap//:zap",
     ],
 )
diff --git a/cmd/server/pactasrv/pacta_version.go b/cmd/server/pactasrv/pacta_version.go
@@ -6,8 +6,6 @@ import (
 
 	api "github.com/RMI/pacta/openapi/pacta"
 	"github.com/RMI/pacta/pacta"
-	"github.com/Silicon-Ally/gqlerr"
-	"go.uber.org/zap"
 )
 
 // Returns a version of the PACTA model by ID
@@ -32,7 +30,7 @@ func (s *Server) CreatePactaVersion(ctx context.Context, request api.CreatePacta
 		Digest:      request.Body.Digest,
 	})
 	if err != nil {
-		return nil, gqlerr.Internal(ctx, "failed to create PACTA version", zap.Error(err))
+		return nil, fmt.Errorf("failed to create PACTA version: %w", err) 
 	}
 	return nil, nil
 }

diff --git a/db/sqldb/BUILD.bazel b/db/sqldb/BUILD.bazel
@@ -14,7 +14,6 @@ go_library(
         "initiative_user.go",
         "owner.go",
         "pacta_version.go",
-        "populate.go",
         "portfolio.go",
         "portfolio_group.go",
         "portfolio_initiative.go",

diff --git a/db/sqldb/analysis.go b/db/sqldb/analysis.go
@@ -106,16 +106,22 @@ func (d *DB) DeleteAnalysis(tx db.Tx, id pacta.AnalysisID) ([]pacta.BlobURI, err
 		if err != nil {
 			return fmt.Errorf("reading analysis: %w", err)
 		}
-		aas, err := d.AnalysisArtifactsForAnalysis(tx, id)
+		rows, err := d.query(tx, `
+			WITH deleted_analysis_artifacts AS (
+				DELETE FROM analysis_artifact
+				WHERE analysis_id = $1 
+				RETURNING blob_id
+			)
+			DELETE FROM blob
+			WHERE id IN (SELECT blob_id FROM deleted_analysis_artifacts)
+			RETURNING blob_uri;
+		`, id)
 		if err != nil {
-			return fmt.Errorf("reading analysis artifacts: %w", err)
+			return fmt.Errorf("reading analysis_artifacts: %w", err)
 		}
-		for _, aa := range aas {
-			buri, err := d.DeleteAnalysisArtifact(tx, aa.ID)
-			if err != nil {
-				return fmt.Errorf("deleting analysis artifact %q: %w", aa.ID, err)
-			}
-			buris = append(buris, buri)
+		buris, err = mapRowsToIDs[pacta.BlobURI]("blob_id", rows)
+		if err != nil {
+			return fmt.Errorf("retrieving analysis_artifacts blob_uris: %w", err)
 		}
 		err = d.exec(tx, `DELETE FROM analysis WHERE id = $1;`, id)
 		if err != nil {
@@ -184,7 +190,7 @@ func rowToAnalysis(row rowScanner) (*pacta.Analysis, error) {
 }
 
 func rowsToAnalyses(rows pgx.Rows) ([]*pacta.Analysis, error) {
-	return allRows("analysis", rows, rowToAnalysis)
+	return mapRows("analysis", rows, rowToAnalysis)
 }
 
 func (db *DB) putAnalysis(tx db.Tx, a *pacta.Analysis) error {

diff --git a/db/sqldb/analysis_artifact.go b/db/sqldb/analysis_artifact.go
@@ -139,7 +139,7 @@ func (db *DB) putAnalysisArtifact(tx db.Tx, a *pacta.AnalysisArtifact) error {
 }
 
 func rowsToAnalysisArtifacts(rows pgx.Rows) ([]*pacta.AnalysisArtifact, error) {
-	return allRows("analysis_artifact", rows, rowToAnalysisArtifact)
+	return mapRows("analysis_artifact", rows, rowToAnalysisArtifact)
 }
 
 func rowToAnalysisArtifact(row rowScanner) (*pacta.AnalysisArtifact, error) {

diff --git a/db/sqldb/audit_log.go b/db/sqldb/audit_log.go
@@ -89,7 +89,7 @@ func (d *DB) CreateAuditLog(tx db.Tx, a *pacta.AuditLog) (pacta.AuditLogID, erro
 }
 
 func rowsToAuditLogs(rows pgx.Rows) ([]*pacta.AuditLog, error) {
-	return allRows("auditLog", rows, rowToAuditLog)
+	return mapRows("auditLog", rows, rowToAuditLog)
 }
 
 func rowToAuditLog(row rowScanner) (*pacta.AuditLog, error) {

diff --git a/db/sqldb/blob.go b/db/sqldb/blob.go
@@ -115,7 +115,7 @@ func (db *DB) putBlob(tx db.Tx, b *pacta.Blob) error {
 }
 
 func rowsToBlobs(rows pgx.Rows) ([]*pacta.Blob, error) {
-	return allRows("blob", rows, rowToBlob)
+	return mapRows("blob", rows, rowToBlob)
 }
 
 func rowToBlob(row rowScanner) (*pacta.Blob, error) {

diff --git a/db/sqldb/cursor.go b/db/sqldb/cursor.go
@@ -8,7 +8,7 @@ import (
 )
 
 func offsetToCursor(i int) db.Cursor {
-	return db.Cursor(fmt.Sprintf("%d", i))
+	return db.Cursor(strconv.Itoa(i))
 }
 
 func offsetFromCursor(c db.Cursor) (int, error) {

diff --git a/db/sqldb/incomplete_upload.go b/db/sqldb/incomplete_upload.go
@@ -174,7 +174,7 @@ func rowToIncompleteUpload(row rowScanner) (*pacta.IncompleteUpload, error) {
 }
 
 func rowsToIncompleteUploads(rows pgx.Rows) ([]*pacta.IncompleteUpload, error) {
-	return allRows("incomplete_upload", rows, rowToIncompleteUpload)
+	return mapRows("incomplete_upload", rows, rowToIncompleteUpload)
 }
 
 func (db *DB) putIncompleteUpload(tx db.Tx, iu *pacta.IncompleteUpload) error {

diff --git a/db/sqldb/initiative.go b/db/sqldb/initiative.go
@@ -174,7 +174,7 @@ func rowToInitiative(row rowScanner) (*pacta.Initiative, error) {
 }
 
 func rowsToInitiatives(rows pgx.Rows) ([]*pacta.Initiative, error) {
-	return allRows("initiative", rows, rowToInitiative)
+	return mapRows("initiative", rows, rowToInitiative)
 }
 
 func (db *DB) putInitiative(tx db.Tx, i *pacta.Initiative) error {

diff --git a/db/sqldb/initiative_invitation.go b/db/sqldb/initiative_invitation.go
@@ -121,7 +121,7 @@ func rowToInitiativeInvitation(row rowScanner) (*pacta.InitiativeInvitation, err
 }
 
 func rowsToInitiativeInvitations(rows pgx.Rows) ([]*pacta.InitiativeInvitation, error) {
-	return allRows("pacta_version", rows, rowToInitiativeInvitation)
+	return mapRows("pacta_version", rows, rowToInitiativeInvitation)
 }
 
 func validateInitiativeInvitationForCreation(ii *pacta.InitiativeInvitation) error {

diff --git a/db/sqldb/initiative_user.go b/db/sqldb/initiative_user.go
@@ -112,5 +112,5 @@ func rowToInitiativeUserRelationship(row rowScanner) (*pacta.InitiativeUserRelat
 }
 
 func rowsToInitiativeUserRelationships(rows pgx.Rows) ([]*pacta.InitiativeUserRelationship, error) {
-	return allRows("initiative_user_relationship", rows, rowToInitiativeUserRelationship)
+	return mapRows("initiative_user_relationship", rows, rowToInitiativeUserRelationship)
 }
diff --git a/db/sqldb/owner.go b/db/sqldb/owner.go
@@ -71,25 +71,49 @@ func (d *DB) ownerByInitiative(tx db.Tx, id pacta.InitiativeID) (*pacta.Owner, e
 }
 
 func (d *DB) GetOrCreateOwnerForUser(tx db.Tx, uID pacta.UserID) (pacta.OwnerID, error) {
-	owner, err := d.ownerByUser(tx, uID)
-	if err == nil {
-		return owner.ID, nil
-	}
-	if !db.IsNotFound(err) {
-		return "", fmt.Errorf("querying owner by user: %w", err)
+	var ownerID pacta.OwnerID
+	err := d.RunOrContinueTransaction(tx, func(tx db.Tx) error {
+		owner, err := d.ownerByUser(tx, uID)
+		if err == nil {
+			ownerID = owner.ID
+			return nil
+		}
+		if !db.IsNotFound(err) {
+			return fmt.Errorf("querying owner by user: %w", err)
+		}
+		ownerID, err = d.createOwner(tx, &pacta.Owner{User: &pacta.User{ID: uID}})
+		if err != nil {
+			return fmt.Errorf("creating owner: %w", err)
+		}
+		return nil
+	})
+	if err != nil {
+		return "", fmt.Errorf("getting or creating owner for initiative: %w", err)
 	}
-	return d.createOwner(tx, &pacta.Owner{User: &pacta.User{ID: uID}})
+	return ownerID, nil
 }
 
 func (d *DB) GetOrCreateOwnerForInitiative(tx db.Tx, iID pacta.InitiativeID) (pacta.OwnerID, error) {
-	owner, err := d.ownerByInitiative(tx, iID)
-	if err == nil {
-		return owner.ID, nil
-	}
-	if !db.IsNotFound(err) {
-		return "", fmt.Errorf("querying owner by user: %w", err)
+	var ownerID pacta.OwnerID
+	err := d.RunOrContinueTransaction(tx, func(tx db.Tx) error {
+		owner, err := d.ownerByInitiative(tx, iID)
+		if err == nil {
+			ownerID = owner.ID
+			return nil
+		}
+		if !db.IsNotFound(err) {
+			return fmt.Errorf("querying owner by user: %w", err)
+		}
+		ownerID, err = d.createOwner(tx, &pacta.Owner{Initiative: &pacta.Initiative{ID: iID}})
+		if err != nil {
+			return fmt.Errorf("creating owner: %w", err)
+		}
+		return nil
+	})
+	if err != nil {
+		return "", fmt.Errorf("getting or creating owner for initiative: %w", err)
 	}
-	return d.createOwner(tx, &pacta.Owner{Initiative: &pacta.Initiative{ID: iID}})
+	return ownerID, nil
 }
 
 func (d *DB) createOwner(tx db.Tx, o *pacta.Owner) (pacta.OwnerID, error) {
@@ -118,7 +142,7 @@ func (d *DB) createOwner(tx db.Tx, o *pacta.Owner) (pacta.OwnerID, error) {
 }
 
 func rowsToOwners(rows pgx.Rows) (map[pacta.OwnerID]*pacta.Owner, error) {
-	owners, err := allRows("owner", rows, rowToOwner)
+	owners, err := mapRows("owner", rows, rowToOwner)
 	if err != nil {
 		return nil, fmt.Errorf("translating rows to owners: %w", err)
 	}

diff --git a/db/sqldb/pacta_version.go b/db/sqldb/pacta_version.go
@@ -54,7 +54,7 @@ func (d *DB) PACTAVersions(tx db.Tx) ([]*pacta.PACTAVersion, error) {
 	if err != nil {
 		return nil, fmt.Errorf("querying pacta_version: %w", err)
 	}
-	pvs, err := allRows("pacta_version", rows, rowToPACTAVersion)
+	pvs, err := mapRows("pacta_version", rows, rowToPACTAVersion)
 	if err != nil {
 		return nil, fmt.Errorf("translating rows to pacta versions: %w", err)
 	}
@@ -152,7 +152,7 @@ func rowToPACTAVersion(row rowScanner) (*pacta.PACTAVersion, error) {
 }
 
 func rowsToPACTAVersions(rows pgx.Rows) ([]*pacta.PACTAVersion, error) {
-	return allRows("pacta_version", rows, rowToPACTAVersion)
+	return mapRows("pacta_version", rows, rowToPACTAVersion)
 }
 
 func (d *DB) putPACTAVersion(tx db.Tx, pv *pacta.PACTAVersion) error {

diff --git a/db/sqldb/populate.go b/db/sqldb/populate.go