Finally remove per-entity columns from EEA (#4305)

Now that the queries aren't using it and the main logic actually uses the central entity instances table, we can drop these. Closes: #4169 Signed-off-by: Juan Antonio Osorio <[email protected]>
mindersec · Aug 29, 2024 · 22600c4 · 22600c4
1 parent e0376fe
commit 22600c4
Show file tree

Hide file tree

Showing 8 changed files with 133 additions and 185 deletions.
diff --git a/database/migrations/000103_eea_rm_per_entities_columns.down.sql b/database/migrations/000103_eea_rm_per_entities_columns.down.sql
@@ -0,0 +1,29 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+ALTER TABLE entity_execution_lock ADD COLUMN IF NOT EXISTS repository_id UUID NOT NULL REFERENCES repositories(id) ON DELETE CASCADE;
+ALTER TABLE entity_execution_lock ADD COLUMN IF NOT EXISTS artifact_id UUID REFERENCES artifacts(id) ON DELETE CASCADE;
+ALTER TABLE entity_execution_lock ADD COLUMN IF NOT EXISTS pull_request_id UUID REFERENCES pull_requests(id) ON DELETE CASCADE;
+
+ALTER TABLE flush_cache ADD COLUMN IF NOT EXISTS repository_id UUID NOT NULL REFERENCES repositories(id) ON DELETE CASCADE;
+ALTER TABLE flush_cache ADD COLUMN IF NOT EXISTS artifact_id UUID REFERENCES artifacts(id) ON DELETE CASCADE;
+ALTER TABLE flush_cache ADD COLUMN IF NOT EXISTS pull_request_id UUID REFERENCES pull_requests(id) ON DELETE CASCADE;
+
+-- make project_id nullable
+ALTER TABLE entity_execution_lock ALTER COLUMN project_id DROP NOT NULL;
+ALTER TABLE flush_cache ALTER COLUMN project_id DROP NOT NULL;
+
+COMMIT;
diff --git a/database/migrations/000103_eea_rm_per_entities_columns.up.sql b/database/migrations/000103_eea_rm_per_entities_columns.up.sql
@@ -0,0 +1,29 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+ALTER TABLE entity_execution_lock DROP COLUMN IF EXISTS repository_id;
+ALTER TABLE entity_execution_lock DROP COLUMN IF EXISTS artifact_id;
+ALTER TABLE entity_execution_lock DROP COLUMN IF EXISTS pull_request_id;
+
+ALTER TABLE flush_cache DROP COLUMN IF EXISTS repository_id;
+ALTER TABLE flush_cache DROP COLUMN IF EXISTS artifact_id;
+ALTER TABLE flush_cache DROP COLUMN IF EXISTS pull_request_id;
+
+-- make project_id not nullable
+ALTER TABLE entity_execution_lock ALTER COLUMN project_id SET NOT NULL;
+ALTER TABLE flush_cache ALTER COLUMN project_id SET NOT NULL;
+
+COMMIT;
diff --git a/internal/db/entity_execution_lock.sql.go b/internal/db/entity_execution_lock.sql.go
diff --git a/internal/db/models.go b/internal/db/models.go
diff --git a/internal/eea/eea.go b/internal/eea/eea.go
@@ -239,33 +239,8 @@ func (e *EEA) FlushAll(ctx context.Context) error {
 	for _, cache := range caches {
 		cache := cache
 
-		projectID := cache.ProjectID
-
-		// ensure that the eiw has a project ID.
-		// If there is no projectID (older minder). Get it from a repo.
-		if !projectID.Valid {
-			if !cache.RepositoryID.Valid {
-				zerolog.Ctx(ctx).Info().Msg("No project nor repo ID provided in entry, skipping")
-				continue
-			}
-			r, err := e.querier.GetRepositoryByID(ctx, cache.RepositoryID.UUID)
-			if err != nil {
-				if errors.Is(err, sql.ErrNoRows) {
-					zerolog.Ctx(ctx).Info().Msg("No project found for repository, skipping")
-					continue
-				}
-				return fmt.Errorf("unable to look up project for repository %s: %w",
-					cache.RepositoryID.UUID, err)
-			}
-
-			projectID = uuid.NullUUID{
-				UUID:  r.ProjectID,
-				Valid: true,
-			}
-		}
-
 		eiw, err := e.buildEntityWrapper(ctx, cache.Entity,
-			cache.RepositoryID, projectID.UUID, cache.ArtifactID, cache.PullRequestID)
+			cache.ProjectID, cache.EntityInstanceID)
 		if err != nil && errors.Is(err, sql.ErrNoRows) {
 			continue
 		} else if err != nil {
@@ -290,17 +265,16 @@ func (e *EEA) FlushAll(ctx context.Context) error {
 func (e *EEA) buildEntityWrapper(
 	ctx context.Context,
 	entity db.Entities,
-	repoID uuid.NullUUID,
 	projID uuid.UUID,
-	artID, prID uuid.NullUUID,
+	entityID uuid.UUID,
 ) (*entities.EntityInfoWrapper, error) {
 	switch entity {
 	case db.EntitiesRepository:
-		return e.buildRepositoryInfoWrapper(ctx, repoID, projID)
+		return e.buildRepositoryInfoWrapper(ctx, entityID, projID)
 	case db.EntitiesArtifact:
-		return e.buildArtifactInfoWrapper(ctx, repoID, projID, artID)
+		return e.buildArtifactInfoWrapper(ctx, entityID, projID)
 	case db.EntitiesPullRequest:
-		return e.buildPullRequestInfoWrapper(ctx, repoID, projID, prID)
+		return e.buildPullRequestInfoWrapper(ctx, entityID, projID)
 	case db.EntitiesBuildEnvironment, db.EntitiesRelease,
 		db.EntitiesPipelineRun, db.EntitiesTaskRun, db.EntitiesBuild:
 		return nil, fmt.Errorf("entity type %q not yet supported", entity)
@@ -311,58 +285,53 @@ func (e *EEA) buildEntityWrapper(
 
 func (e *EEA) buildRepositoryInfoWrapper(
 	ctx context.Context,
-	repoID uuid.NullUUID,
+	repoID uuid.UUID,
 	projID uuid.UUID,
 ) (*entities.EntityInfoWrapper, error) {
-	providerID, r, err := getRepository(ctx, e.querier, projID, repoID.UUID)
+	providerID, r, err := getRepository(ctx, e.querier, projID, repoID)
 	if err != nil {
 		return nil, fmt.Errorf("error getting repository: %w", err)
 	}
 
 	return entities.NewEntityInfoWrapper().
 		WithRepository(r).
-		WithRepositoryID(repoID.UUID).
+		WithRepositoryID(repoID).
 		WithProjectID(projID).
 		WithProviderID(providerID), nil
 }
 
 func (e *EEA) buildArtifactInfoWrapper(
 	ctx context.Context,
-	repoID uuid.NullUUID,
+	artID uuid.UUID,
 	projID uuid.UUID,
-	artID uuid.NullUUID,
 ) (*entities.EntityInfoWrapper, error) {
-	providerID, a, err := artifacts.GetArtifact(ctx, e.querier, projID, artID.UUID)
+	providerID, a, err := artifacts.GetArtifact(ctx, e.querier, projID, artID)
 	if err != nil {
 		return nil, fmt.Errorf("error getting artifact with versions: %w", err)
 	}
 
 	eiw := entities.NewEntityInfoWrapper().
 		WithProjectID(projID).
 		WithArtifact(a).
-		WithArtifactID(artID.UUID).
+		WithArtifactID(artID).
 		WithProviderID(providerID)
-	if repoID.Valid {
-		eiw.WithRepositoryID(repoID.UUID)
-	}
 	return eiw, nil
 }
 
 func (e *EEA) buildPullRequestInfoWrapper(
 	ctx context.Context,
-	repoID uuid.NullUUID,
+	prID uuid.UUID,
 	projID uuid.UUID,
-	prID uuid.NullUUID,
 ) (*entities.EntityInfoWrapper, error) {
-	providerID, pr, err := getPullRequest(ctx, e.querier, projID, repoID.UUID, prID.UUID)
+	providerID, repoID, pr, err := getPullRequest(ctx, e.querier, projID, prID)
 	if err != nil {
 		return nil, fmt.Errorf("error getting pull request: %w", err)
 	}
 
 	return entities.NewEntityInfoWrapper().
-		WithRepositoryID(repoID.UUID).
+		WithRepositoryID(repoID).
 		WithProjectID(projID).
 		WithPullRequest(pr).
-		WithPullRequestID(prID.UUID).
+		WithPullRequestID(prID).
 		WithProviderID(providerID), nil
 }