Fixes 4741: associate snapshots to templates directly (#836)

* Fixes 4741: associate snapshots to templates directly * make snapshot_uuid not null, insert uuids on create, fix tests * lint * fix migration * remove snaps from trc table before deleting
content-services · Oct 21, 2024 · 629a778 · 629a778
1 parent 3eac460
commit 629a778
Show file tree

Hide file tree

Showing 21 changed files with 411 additions and 111 deletions.
diff --git a/api/docs.go b/api/docs.go
@@ -3238,6 +3238,24 @@ const docTemplate = `{
                         "description": "Search through snapshots by repository name.",
                         "name": "repository_search",
                         "in": "query"
+                    },
+                    {
+                        "type": "string",
+                        "description": "Sort the response data based on specific snapshot parameters. Sort criteria can include ` + "`" + `repository_name` + "`" + ` or ` + "`" + `created_at` + "`" + `.",
+                        "name": "sort_by",
+                        "in": "query"
+                    },
+                    {
+                        "type": "integer",
+                        "description": "Starting point for retrieving a subset of results. Determines how many items to skip from the beginning of the result set. Default value:` + "`" + `0` + "`" + `.",
+                        "name": "offset",
+                        "in": "query"
+                    },
+                    {
+                        "type": "integer",
+                        "description": "Number of items to include in response. Use it to control the number of items, particularly when dealing with large datasets. Default value: ` + "`" + `100` + "`" + `.",
+                        "name": "limit",
+                        "in": "query"
                     }
                 ],
                 "responses": {

diff --git a/api/openapi.json b/api/openapi.json
@@ -5806,6 +5806,30 @@
                         "schema": {
                             "type": "string"
                         }
+                    },
+                    {
+                        "description": "Sort the response data based on specific snapshot parameters. Sort criteria can include `repository_name` or `created_at`.",
+                        "in": "query",
+                        "name": "sort_by",
+                        "schema": {
+                            "type": "string"
+                        }
+                    },
+                    {
+                        "description": "Starting point for retrieving a subset of results. Determines how many items to skip from the beginning of the result set. Default value:`0`.",
+                        "in": "query",
+                        "name": "offset",
+                        "schema": {
+                            "type": "integer"
+                        }
+                    },
+                    {
+                        "description": "Number of items to include in response. Use it to control the number of items, particularly when dealing with large datasets. Default value: `100`.",
+                        "in": "query",
+                        "name": "limit",
+                        "schema": {
+                            "type": "integer"
+                        }
                     }
                 ],
                 "responses": {

diff --git a/db/migrations.latest b/db/migrations.latest
@@ -1 +1,2 @@
-20241011084507
+20241018154315
+
diff --git a/db/migrations/20241018154315_associate_snaps_to_templates.down.sql b/db/migrations/20241018154315_associate_snaps_to_templates.down.sql
@@ -0,0 +1,8 @@
+BEGIN;
+
+ALTER TABLE templates_repository_configurations DROP COLUMN IF EXISTS snapshot_uuid;
+
+ALTER TABLE templates_repository_configurations
+DROP CONSTRAINT IF EXISTS fk_templates_repository_configurations_snapshots;
+
+COMMIT;
diff --git a/db/migrations/20241018154315_associate_snaps_to_templates.up.sql b/db/migrations/20241018154315_associate_snaps_to_templates.up.sql
@@ -0,0 +1,65 @@
+BEGIN;
+
+ALTER TABLE templates_repository_configurations ADD COLUMN IF NOT EXISTS snapshot_uuid UUID;
+
+-- migrate snapshots for use_latest templates
+UPDATE templates_repository_configurations trc
+SET snapshot_uuid = (
+    SELECT rc.last_snapshot_uuid
+    FROM repository_configurations rc
+             JOIN templates t ON t.uuid = trc.template_uuid
+    WHERE rc.uuid = trc.repository_configuration_uuid
+      AND t.use_latest = true
+      AND rc.last_snapshot_uuid IS NOT NULL
+)
+WHERE trc.template_uuid IN (
+    SELECT t.uuid
+    FROM templates t
+    WHERE t.use_latest = true
+);
+
+-- migrate snapshots for templates with a snapshot date
+UPDATE templates_repository_configurations trc
+SET snapshot_uuid = (
+    SELECT closest_snapshots.uuid
+    FROM (
+             (SELECT s.uuid, s.created_at
+              FROM snapshots s
+                       JOIN templates t ON t.uuid = trc.template_uuid
+              WHERE s.repository_configuration_uuid = trc.repository_configuration_uuid
+                AND t.use_latest = false
+                AND s.created_at <= t.date
+              ORDER BY s.created_at DESC
+                  LIMIT 1)
+
+             UNION
+
+             (SELECT s.uuid, s.created_at
+              FROM snapshots s
+                       JOIN templates t ON t.uuid = trc.template_uuid
+              WHERE s.repository_configuration_uuid = trc.repository_configuration_uuid
+                AND t.use_latest = false
+                AND s.created_at > t.date
+              ORDER BY s.created_at ASC
+                  LIMIT 1)
+         ) AS closest_snapshots
+    ORDER BY closest_snapshots.created_at ASC
+    LIMIT 1
+)
+WHERE trc.template_uuid IN (
+    SELECT t.uuid
+    FROM templates t
+    WHERE t.use_latest = false
+    );
+
+DELETE FROM templates_repository_configurations WHERE snapshot_uuid IS NULL;
+
+ALTER TABLE templates_repository_configurations ALTER COLUMN snapshot_uuid SET NOT NULL;
+
+ALTER TABLE templates_repository_configurations
+DROP CONSTRAINT IF EXISTS fk_templates_repository_configurations_snapshots,
+ADD CONSTRAINT fk_templates_repository_configurations_snapshots
+FOREIGN KEY (snapshot_uuid) REFERENCES snapshots(uuid)
+ON DELETE RESTRICT;
+
+COMMIT;
diff --git a/pkg/dao/interfaces.go b/pkg/dao/interfaces.go
@@ -187,9 +187,11 @@ type TemplateDao interface {
 	Update(ctx context.Context, orgID string, uuid string, templParams api.TemplateUpdateRequest) (api.TemplateResponse, error)
 	GetRepoChanges(ctx context.Context, templateUUID string, newRepoConfigUUIDs []string) ([]string, []string, []string, []string, error)
 	GetDistributionHref(ctx context.Context, templateUUID string, repoConfigUUID string) (string, error)
-	UpdateDistributionHrefs(ctx context.Context, templateUUID string, repoUUIDs []string, repoDistributionMap map[string]string) error
+	UpdateDistributionHrefs(ctx context.Context, templateUUID string, repoUUIDs []string, snapshots []models.Snapshot, repoDistributionMap map[string]string) error
 	DeleteTemplateRepoConfigs(ctx context.Context, templateUUID string, keepRepoConfigUUIDs []string) error
 	UpdateLastUpdateTask(ctx context.Context, taskUUID string, orgID string, templateUUID string) error
 	UpdateLastError(ctx context.Context, orgID string, templateUUID string, lastUpdateSnapshotError string) error
 	SetEnvironmentCreated(ctx context.Context, templateUUID string) error
+	UpdateSnapshots(ctx context.Context, templateUUID string, repoUUIDs []string, snapshots []models.Snapshot) error
+	DeleteTemplateSnapshot(ctx context.Context, snapshotUUID string) error
 }
diff --git a/pkg/dao/rpms_test.go b/pkg/dao/rpms_test.go
@@ -1301,12 +1301,12 @@ func (s *RpmSuite) TestListRpmsForTemplates() {
 	res := s.tx.Where("org_id = ?", orgId).First(&repoConfig)
 	require.NoError(s.T(), res.Error)
 
-	_, err = seeds.SeedSnapshots(s.tx, repoConfig.UUID, 1)
+	snaps, err := seeds.SeedSnapshots(s.tx, repoConfig.UUID, 1)
 	require.NoError(s.T(), err)
 	res = s.tx.Model(&models.Snapshot{}).Where("repository_configuration_uuid = ?", repoConfig.UUID).Update("version_href", hrefs[0])
 	require.NoError(s.T(), res.Error)
 
-	_, err = seeds.SeedTemplates(s.tx, 1, seeds.TemplateSeedOptions{OrgID: orgId, RepositoryConfigUUIDs: []string{repoConfig.UUID}})
+	_, err = seeds.SeedTemplates(s.tx, 1, seeds.TemplateSeedOptions{OrgID: orgId, RepositoryConfigUUIDs: []string{repoConfig.UUID}, Snapshots: []models.Snapshot{snaps[0]}})
 	require.NoError(s.T(), err)
 	template := models.Template{}
 	res = s.tx.Where("org_id = ?", orgId).First(&template)
@@ -1347,13 +1347,13 @@ func (s *RpmSuite) TestListErrataForTemplates() {
 	res := s.tx.Where("org_id = ?", orgId).First(&repoConfig)
 	require.NoError(s.T(), res.Error)
 
-	_, err = seeds.SeedSnapshots(s.tx, repoConfig.UUID, 1)
+	snaps, err := seeds.SeedSnapshots(s.tx, repoConfig.UUID, 1)
 	require.NoError(s.T(), err)
 
 	res = s.tx.Model(&models.Snapshot{}).Where("repository_configuration_uuid = ?", repoConfig.UUID).Update("version_href", hrefs[0])
 	require.NoError(s.T(), res.Error)
 
-	templates, err := seeds.SeedTemplates(s.tx, 1, seeds.TemplateSeedOptions{OrgID: orgId, RepositoryConfigUUIDs: []string{repoConfig.UUID}})
+	templates, err := seeds.SeedTemplates(s.tx, 1, seeds.TemplateSeedOptions{OrgID: orgId, RepositoryConfigUUIDs: []string{repoConfig.UUID}, Snapshots: []models.Snapshot{snaps[0]}})
 	require.NoError(s.T(), err)
 	template := templates[0]
 

diff --git a/pkg/dao/snapshots.go b/pkg/dao/snapshots.go
@@ -134,57 +134,40 @@ func (sDao *snapshotDaoImpl) ListByTemplate(
 		return api.SnapshotCollectionResponse{}, 0, err
 	}
 
-	// Get snapshots for template date
-	var date time.Time
-	if template.UseLatest {
-		date = time.Now()
-	} else {
-		date = template.Date
-	}
-	snapshotsForTemplateDate, err := sDao.FetchSnapshotsByDateAndRepository(ctx, orgID, api.ListSnapshotByDateRequest{
-		RepositoryUUIDS: template.RepositoryUUIDS,
-		Date:            date,
-	})
-	if err != nil {
-		return api.SnapshotCollectionResponse{}, totalSnaps, err
-	}
-
 	// Repository search/filter and ordering
 	sortMap := map[string]string{
 		"repository_name": "repo_name",
 		"created_at":      "snapshots.created_at",
 	}
 	order := convertSortByToSQL(paginationData.SortBy, sortMap, "repo_name ASC")
+
+	baseQuery := readableSnapshots(sDao.db.WithContext(ctx), orgID).
+		Joins("JOIN templates_repository_configurations ON templates_repository_configurations.snapshot_uuid = snapshots.uuid").
+		Where("templates_repository_configurations.template_uuid = ?", template.UUID).
+		Where("repository_configurations.name ILIKE ?", fmt.Sprintf("%%%s%%", repositorySearch))
+
+	countQuery := baseQuery.
+		Count(&totalSnaps)
+	if countQuery.Error != nil {
+		return api.SnapshotCollectionResponse{}, totalSnaps, countQuery.Error
+	}
+
 	var filteredSnaps []models.Snapshot
-	query := readableSnapshots(sDao.db.WithContext(ctx), orgID).
+	listQuery := baseQuery.
 		Select("snapshots.*, STRING_AGG(repository_configurations.name, '') as repo_name").
-		Where("repository_configuration_uuid IN ?", template.RepositoryUUIDS).
-		Where("repository_configurations.name ILIKE ?", fmt.Sprintf("%%%s%%", repositorySearch)).
 		Group("snapshots.uuid").
-		Group("snapshots.repository_configuration_uuid").
+		Limit(paginationData.Limit).
+		Offset(paginationData.Offset).
 		Order(order).
 		Find(&filteredSnaps)
-	if query.Error != nil {
-		return api.SnapshotCollectionResponse{}, totalSnaps, query.Error
-	}
-	snapsApi := snapshotConvertToResponses(filteredSnaps, pulpContentPath)
-
-	// Intersect ordered snapshots and snapshots for template date
-	for _, snap := range snapsApi {
-		indx := slices.IndexFunc(snapshotsForTemplateDate.Data, func(c api.SnapshotForDate) bool {
-			return c.Match != nil && c.Match.UUID == snap.UUID
-		})
-		if indx != -1 {
-			snaps = append(snaps, snap)
-		}
+	if listQuery.Error != nil {
+		return api.SnapshotCollectionResponse{}, totalSnaps, listQuery.Error
 	}
 
-	totalSnaps = int64(len(snaps))
+	snaps = snapshotConvertToResponses(filteredSnaps, pulpContentPath)
 	if totalSnaps == 0 {
 		return api.SnapshotCollectionResponse{Data: []api.SnapshotResponse{}}, totalSnaps, nil
 	}
-	start, end := min(paginationData.Offset, len(snaps)), min(paginationData.Offset+paginationData.Limit, len(snaps))
-	snaps = snaps[start:end]
 
 	return api.SnapshotCollectionResponse{Data: snaps}, totalSnaps, nil
 }

diff --git a/pkg/dao/snapshots_test.go b/pkg/dao/snapshots_test.go
@@ -164,6 +164,7 @@ func (s *SnapshotsSuite) createTemplate(orgID string, rConfigs ...models.Reposit
 	var repoUUIDs []string
 	for _, repo := range rConfigs {
 		repoUUIDs = append(repoUUIDs, repo.UUID)
+		s.createSnapshot(repo)
 	}
 
 	timeNow := time.Now()
@@ -632,7 +633,7 @@ func (s *SnapshotsSuite) TestListByTemplate() {
 	repoConfig2 := s.createRepositoryWithPrefix("First")
 	redhatRepo := s.createRedhatRepository()
 	template := s.createTemplate(repoConfig.OrgID, repoConfig, repoConfig2, redhatRepo)
-	template.RepositoryUUIDS = []string{repoConfig.UUID, repoConfig2.UUID, redhatRepo.UUID, uuid2.NewString()}
+	template.RepositoryUUIDS = []string{repoConfig.UUID, repoConfig2.UUID, redhatRepo.UUID}
 
 	baseTime := time.Now()
 	t1b := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(-time.Hour*30)) // Before Date
@@ -643,9 +644,9 @@ func (s *SnapshotsSuite) TestListByTemplate() {
 	t2a := s.createSnapshotAtSpecifiedTime(repoConfig2, baseTime.Add(time.Hour*70))  // After Date
 	t2aa := s.createSnapshotAtSpecifiedTime(repoConfig2, baseTime.Add(time.Hour*90)) // After Date
 
-	s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*600)) // Before Date
-	s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*200)) // Before Date
-	s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*100)) // Closest to Target Date
+	s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*600))       // Before Date
+	s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*200))       // Before Date
+	t3 := s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*100)) // Closest to Target Date
 
 	const NonRedHatRepoSearch = "to"
 	pageData := api.PaginationData{
@@ -654,6 +655,10 @@ func (s *SnapshotsSuite) TestListByTemplate() {
 		SortBy: "repository_name:desc",
 	}
 
+	tDao := templateDaoImpl{db: tx}
+	err := tDao.UpdateSnapshots(context.Background(), template.UUID, template.RepositoryUUIDS, []models.Snapshot{t1, t2, t3})
+	assert.NoError(t, err)
+
 	snapshots, totalSnapshots, err := sDao.ListByTemplate(context.Background(), repoConfig.OrgID, template, NonRedHatRepoSearch, pageData)
 
 	assert.NoError(t, err)
@@ -675,33 +680,6 @@ func (s *SnapshotsSuite) TestListByTemplate() {
 	assert.Equal(t, repoConfig2.UUID, snapshots.Data[1].RepositoryUUID)
 }
 
-func (s *SnapshotsSuite) TestListByTemplateNoRepos() {
-	t := s.T()
-	tx := s.tx
-
-	mockPulpClient := pulp_client.NewMockPulpClient(t)
-	sDao := snapshotDaoImpl{db: tx, pulpClient: mockPulpClient}
-
-	mockPulpClient.On("GetContentPath", context.Background()).Return(testContentPath, nil)
-
-	repoConfig := s.createRepository()
-	template := s.createTemplate(repoConfig.OrgID, repoConfig)
-	// Invalidate template repo UUIDs
-	template.RepositoryUUIDS = []string{uuid2.NewString()}
-
-	pageData := api.PaginationData{
-		Limit:  100,
-		Offset: 0,
-		SortBy: "created_at:desc",
-	}
-
-	snapshots, totalSnapshots, err := sDao.ListByTemplate(context.Background(), repoConfig.OrgID, template, "", pageData)
-
-	assert.NoError(t, err)
-	assert.Equal(t, 0, len(snapshots.Data))
-	assert.Equal(t, int64(0), totalSnapshots)
-}
-
 func (s *SnapshotsSuite) TestListByTemplateWithPagination() {
 	t := s.T()
 	tx := s.tx
@@ -715,13 +693,12 @@ func (s *SnapshotsSuite) TestListByTemplateWithPagination() {
 	repoConfig2 := s.createRepositoryWithPrefix("First")
 	redhatRepo := s.createRedhatRepository()
 	template := s.createTemplate(repoConfig.OrgID, repoConfig, repoConfig2, redhatRepo)
-	template.RepositoryUUIDS = []string{repoConfig.UUID, repoConfig2.UUID, redhatRepo.UUID, uuid2.NewString()}
+	template.RepositoryUUIDS = []string{repoConfig.UUID, repoConfig2.UUID, redhatRepo.UUID}
 
 	baseTime := time.Now()
-	target := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime)
-	s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(time.Hour*30))
-	s.createSnapshotAtSpecifiedTime(repoConfig2, baseTime.Add(time.Hour*30))
-	s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*100))
+	t1 := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(-time.Hour*30))
+	t2 := s.createSnapshotAtSpecifiedTime(repoConfig2, baseTime)
+	t3 := s.createSnapshotAtSpecifiedTime(redhatRepo, baseTime.Add(-time.Hour*100))
 
 	// First call
 	pageData := api.PaginationData{
@@ -730,6 +707,10 @@ func (s *SnapshotsSuite) TestListByTemplateWithPagination() {
 		SortBy: "created_at:desc",
 	}
 
+	tDao := templateDaoImpl{db: tx}
+	err := tDao.UpdateSnapshots(context.Background(), template.UUID, template.RepositoryUUIDS, []models.Snapshot{t1, t2, t3})
+	assert.NoError(t, err)
+
 	snapshots, totalSnapshots, err := sDao.ListByTemplate(context.Background(), repoConfig.OrgID, template, "", pageData)
 
 	assert.NoError(t, err)
@@ -738,7 +719,7 @@ func (s *SnapshotsSuite) TestListByTemplateWithPagination() {
 
 	// target
 	assert.True(t, bytes.Contains([]byte(snapshots.Data[0].RepositoryName), []byte("Last")))
-	assert.Equal(t, target.Base.CreatedAt.Day(), snapshots.Data[0].CreatedAt.Day())
+	assert.Equal(t, t1.Base.CreatedAt.Day(), snapshots.Data[0].CreatedAt.Day())
 	assert.Equal(t, repoConfig.UUID, snapshots.Data[0].RepositoryUUID)
 
 	// Second call (test for no nil snapshot overflow)