Skip to content

Commit

Permalink
Fixes 4740: select correct snapshots for date selection (#841)
Browse files Browse the repository at this point in the history 
Fixes 4740: select only a single snapshot per repo for template
  • Loading branch information
@jlsherrill
jlsherrill authored Oct 11, 2024
1 parent 533684f commit e1e3ad9
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 19 deletions.
45 changes: 33 additions & 12 deletions pkg/dao/snapshots.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,19 +359,40 @@ func (sDao *snapshotDaoImpl) FetchSnapshotByVersionHref(ctx context.Context, rep

func (sDao *snapshotDaoImpl) FetchSnapshotsModelByDateAndRepository(ctx context.Context, orgID string, request api.ListSnapshotByDateRequest) ([]models.Snapshot, error) {
snaps := []models.Snapshot{}
query := sDao.db.WithContext(ctx).
Model(&models.Snapshot{}).
Preload("RepositoryConfiguration").
InnerJoins("INNER JOIN repository_configurations ON snapshots.repository_configuration_uuid = repository_configurations.uuid").
Where("repository_configuration_uuid IN ?", request.RepositoryUUIDS).
Where("repository_configurations.org_id IN ?", []string{orgID, config.RedHatOrg}).
Order(fmt.Sprintf("ABS(EXTRACT(EPOCH FROM snapshots.created_at) - EXTRACT(EPOCH FROM TIMESTAMP '%s')) ASC", request.Date.Format(time.RFC3339))).
Group("snapshots.uuid").
Group("snapshots.repository_configuration_uuid").
Find(&snaps)
date := request.Date.Format(time.RFC3339)

// finds the snapshot for each repo that is just before (or equal to) our date
beforeQuery := sDao.db.WithContext(ctx).Raw(`
SELECT DISTINCT ON (s.repository_configuration_uuid) s.uuid
FROM snapshots s
INNER JOIN repository_configurations ON s.repository_configuration_uuid = repository_configurations.uuid
WHERE s.repository_configuration_uuid IN ?
AND repository_configurations.org_id IN ?
AND date_trunc('second', s.created_at::timestamp) <= ?
ORDER BY s.repository_configuration_uuid, s.created_at DESC
`, request.RepositoryUUIDS, []string{orgID, config.RedHatOrg}, date)

// finds the snapshot for each repo that is the first one after our date
afterQuery := sDao.db.WithContext(ctx).Raw(`SELECT DISTINCT ON (s.repository_configuration_uuid) s.uuid
FROM snapshots s
INNER JOIN repository_configurations ON s.repository_configuration_uuid = repository_configurations.uuid
WHERE s.repository_configuration_uuid IN ?
AND repository_configurations.org_id IN ?
AND date_trunc('second', s.created_at::timestamp) > ?
ORDER BY s.repository_configuration_uuid, s.created_at ASC
`, request.RepositoryUUIDS, []string{orgID, config.RedHatOrg}, date)
// For each repo, pick the oldest of this combined set (ideally the one just before our date, if that doesn't exist, the one after)
combined := sDao.db.WithContext(ctx).Raw(`
select DISTINCT ON (s2.repository_configuration_uuid) s2.uuid
from snapshots s2
where s2.uuid in ((?) UNION (?))
ORDER BY s2.repository_configuration_uuid, s2.created_at ASC
`, beforeQuery, afterQuery)

result := sDao.db.WithContext(ctx).Model(&models.Snapshot{}).Where("uuid in (?)", combined).Find(&snaps)

if query.Error != nil {
return nil, query.Error
if result.Error != nil {
return nil, fmt.Errorf("could not query snapshots for date %w", result.Error)
}
return snaps, nil
}
Expand Down
63 changes: 56 additions & 7 deletions pkg/dao/snapshots_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,11 +478,11 @@ func (s *SnapshotsSuite) TestFetchSnapshotsByDateAndRepository() {
baseTime := time.Now()
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(-time.Hour*30)) // Before Date
second := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime) // Target Date
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(time.Hour*30)) // After Date
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(time.Hour*1)) // After Date

request := api.ListSnapshotByDateRequest{}

request.Date = second.Base.CreatedAt
request.Date = second.Base.CreatedAt.Add(time.Minute * 31)

request.RepositoryUUIDS = []string{repoConfig.UUID}

Expand All @@ -496,6 +496,54 @@ func (s *SnapshotsSuite) TestFetchSnapshotsByDateAndRepository() {
assert.NotEmpty(t, response.Data[0].Match.URL)
}

func (s *SnapshotsSuite) TestFetchSnapshotsModelByDateAndRepositoryNew() {
t := s.T()
tx := s.tx

repoConfig := s.createRepository()
baseTime := time.Now()
first := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(-time.Hour*30)) // Before Date
second := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime) // Target Date
third := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(time.Hour*1)) // After Date

sDao := GetSnapshotDao(tx)
// Exact match to second
response, err := sDao.FetchSnapshotsModelByDateAndRepository(context.Background(), repoConfig.OrgID, api.ListSnapshotByDateRequest{
RepositoryUUIDS: []string{repoConfig.UUID},
Date: second.Base.CreatedAt.Add(time.Second * 1),
})
assert.NoError(t, err)
assert.Equal(t, 1, len(response))
assert.Equal(t, second.Base.UUID, response[0].UUID)

// 31 minutes after should still use second
response, err = sDao.FetchSnapshotsModelByDateAndRepository(context.Background(), repoConfig.OrgID, api.ListSnapshotByDateRequest{
RepositoryUUIDS: []string{repoConfig.UUID},
Date: second.Base.CreatedAt.Add(time.Minute * 31),
})
assert.NoError(t, err)
assert.Equal(t, 1, len(response))
assert.Equal(t, second.Base.UUID, response[0].UUID)

// 1 minute before should use first
response, err = sDao.FetchSnapshotsModelByDateAndRepository(context.Background(), repoConfig.OrgID, api.ListSnapshotByDateRequest{
RepositoryUUIDS: []string{repoConfig.UUID},
Date: second.Base.CreatedAt.Add(time.Minute * -1),
})
assert.NoError(t, err)
assert.Equal(t, 1, len(response))
assert.Equal(t, first.Base.UUID, response[0].UUID)

// 2 hours after should use third
response, err = sDao.FetchSnapshotsModelByDateAndRepository(context.Background(), repoConfig.OrgID, api.ListSnapshotByDateRequest{
RepositoryUUIDS: []string{repoConfig.UUID},
Date: second.Base.CreatedAt.Add(time.Minute * 120),
})
assert.NoError(t, err)
assert.Equal(t, 1, len(response))
assert.Equal(t, third.Base.UUID, response[0].UUID)
}

func (s *SnapshotsSuite) TestFetchSnapshotsByDateAndRepositoryMulti() {
t := s.T()
tx := s.tx
Expand All @@ -509,9 +557,9 @@ func (s *SnapshotsSuite) TestFetchSnapshotsByDateAndRepositoryMulti() {
redhatRepo := s.createRedhatRepository()

baseTime := time.Now()
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(-time.Hour*30)) // Before Date
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(-time.Hour*24)) // Before Date
target1 := s.createSnapshotAtSpecifiedTime(repoConfig, baseTime) // Closest to Target Date
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(time.Hour*30)) // After Date
s.createSnapshotAtSpecifiedTime(repoConfig, baseTime.Add(time.Hour*24)) // After Date

target2 := s.createSnapshotAtSpecifiedTime(repoConfig2, baseTime.Add(time.Hour*30)) // Target Date with IsAfter = true
s.createSnapshotAtSpecifiedTime(repoConfig2, baseTime.Add(time.Hour*70)) // After Date
Expand Down Expand Up @@ -541,17 +589,18 @@ func (s *SnapshotsSuite) TestFetchSnapshotsByDateAndRepositoryMulti() {
// target 1
assert.Equal(t, false, response[0].IsAfter)
assert.Equal(t, target1.Base.UUID, response[0].Match.UUID)
assert.Equal(t, target1.Base.CreatedAt.Day(), response[0].Match.CreatedAt.Day())
// We have to round to the nearest second as go times are at a different precision than postgresql times and won't be exactly equal
assert.Equal(t, target1.Base.CreatedAt.Round(time.Second), response[0].Match.CreatedAt.Round(time.Second))

// target 2
assert.Equal(t, true, response[1].IsAfter)
assert.Equal(t, target2.Base.UUID, response[1].Match.UUID)
assert.Equal(t, target2.Base.CreatedAt.Day(), response[1].Match.CreatedAt.Day())
assert.Equal(t, target2.Base.CreatedAt.Round(time.Second), response[1].Match.CreatedAt.Round(time.Second))

// target 3 < RedHat repo before the expected date
assert.Equal(t, false, response[2].IsAfter)
assert.Equal(t, target3.Base.UUID, response[2].Match.UUID)
assert.Equal(t, target3.Base.CreatedAt.Day(), response[2].Match.CreatedAt.Day())
assert.Equal(t, target3.Base.CreatedAt.Round(time.Second), response[2].Match.CreatedAt.Round(time.Second))

// target 4 < RandomUUID Expect empty state
assert.Equal(t, randomUUID.String(), response[3].RepositoryUUID)
Expand Down

0 comments on commit e1e3ad9

Please sign in to comment.