From b107544a6cdadea20b7f2fc48e22b96b73d6c03d Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Mon, 26 Feb 2024 14:45:59 -0500 Subject: [PATCH 1/7] Add rpm list api --- internal/test/integration/rpm_test.go | 58 ++++++++++++++++++--- pkg/tangy/interface.go | 1 + pkg/tangy/rpm.go | 75 +++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 7 deletions(-) diff --git a/internal/test/integration/rpm_test.go b/internal/test/integration/rpm_test.go index 434f822..24916be 100644 --- a/internal/test/integration/rpm_test.go +++ b/internal/test/integration/rpm_test.go @@ -30,14 +30,11 @@ const testRepoName = "zoo" const testRepoURL = "https://rverdile.fedorapeople.org/dummy-repos/comps/repo1/" const testRepoURLTwo = "https://rverdile.fedorapeople.org/dummy-repos/comps/repo2/" -func (r *RpmSuite) CreateTestRepository(t *testing.T) { - domainName := RandStringBytes(10) - r.domainName = domainName - - _, err := r.client.LookupOrCreateDomain(domainName) +func (r *RpmSuite) CreateTestRepository(t *testing.T, repoName string) { + _, err := r.client.LookupOrCreateDomain(r.domainName) require.NoError(t, err) - repoHref, remoteHref, err := r.client.CreateRepository(domainName, testRepoName, testRepoURL) + repoHref, remoteHref, err := r.client.CreateRepository(r.domainName, repoName, testRepoURL) require.NoError(t, err) r.repoHref = repoHref @@ -78,7 +75,10 @@ func TestRpmSuite(t *testing.T) { r := RpmSuite{} r.client = &rpmZest r.tangy = ta - r.CreateTestRepository(t) + + r.domainName = RandStringBytes(10) + + r.CreateTestRepository(t, testRepoName) // Get first version href resp, err := r.client.GetRpmRepositoryByName(r.domainName, testRepoName) @@ -244,6 +244,50 @@ func (r *RpmSuite) TestRpmRepositoryVersionEnvironmentSearch() { assert.Len(r.T(), search, 0) } +func (r *RpmSuite) TestRpmRepositoryVersionPackageListNameFilter() { + resp, err := r.client.GetRpmRepositoryByName(r.domainName, testRepoName) + require.NoError(r.T(), err) + firstVersionHref := resp.LatestVersionHref + require.NotNil(r.T(), firstVersionHref) + + // exact match + singleList, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "bear"}, tangy.PageOptions{}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), singleList) + + // partial match + singleList, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "bea"}, tangy.PageOptions{}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), singleList) + + // no match + singleList, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "wal"}, tangy.PageOptions{}) + require.NoError(r.T(), err) + assert.Empty(r.T(), singleList) +} + +// RpmRepositoryVersionPackageList +func (r *RpmSuite) TestRpmRepositoryVersionPackageListNoDuplicates() { + firstVersionHref := r.firstVersionHref + + otherRepoName := "Same Repo url " + r.CreateTestRepository(r.T(), otherRepoName) + resp, err := r.client.GetRpmRepositoryByName(r.domainName, otherRepoName) + require.NoError(r.T(), err) + secondVersionHref := resp.LatestVersionHref + require.NotNil(r.T(), firstVersionHref) + + doubleList, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, *secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), doubleList) + + singleList, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), singleList) + + assert.Equal(r.T(), singleList, doubleList) +} + func RandStringBytes(n int) string { const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" b := make([]byte, n) diff --git a/pkg/tangy/interface.go b/pkg/tangy/interface.go index 20a4475..ecaeb55 100644 --- a/pkg/tangy/interface.go +++ b/pkg/tangy/interface.go @@ -55,6 +55,7 @@ type Tangy interface { RpmRepositoryVersionPackageSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmPackageSearch, error) RpmRepositoryVersionPackageGroupSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmPackageGroupSearch, error) RpmRepositoryVersionEnvironmentSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmEnvironmentSearch, error) + RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, error) Close() } diff --git a/pkg/tangy/rpm.go b/pkg/tangy/rpm.go index 2321796..9338f55 100644 --- a/pkg/tangy/rpm.go +++ b/pkg/tangy/rpm.go @@ -37,6 +37,25 @@ type RpmEnvironmentSearch struct { Description string } +type RpmListItem struct { + Id string + Name string // The rpm package name + Arch string // The Architecture of the rpm + Version string // The version of the rpm + Release string // The release of the rpm + Epoch string // The epoch of the rpm + Summary string // The summary of the rpm +} + +type PageOptions struct { + Offset int32 + Limit int32 +} + +type RpmListFilters struct { + Name string +} + // RpmRepositoryVersionPackageSearch search for RPMs, by name, associated to repository hrefs, returning an amount up to limit func (t *tangyImpl) RpmRepositoryVersionPackageSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmPackageSearch, error) { if len(hrefs) == 0 { @@ -211,6 +230,62 @@ func buildSearchQuery(queryFragment string, search string, limit int, repository return query } +// RpmRepositoryVersionPackageSearch search for RPMs, by name, associated to repository hrefs, returning an amount up to limit +func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, error) { + if len(hrefs) == 0 { + return []RpmListItem{}, nil + } + + conn, err := t.pool.Acquire(ctx) + if err != nil { + return nil, err + } + defer conn.Release() + + if pageOpts.Limit == 0 { + pageOpts.Limit = DefaultLimit + } + + repositoryIDs, versions, err := parseRepositoryVersionHrefs(hrefs) + if err != nil { + return nil, fmt.Errorf("error parsing repository version hrefs: %w", err) + } + + query := `SELECT rp.content_ptr_id as id, rp.name, rp.version, rp.arch, rp.release, rp.epoch, rp.summary + FROM rpm_package rp WHERE rp.content_ptr_id IN (` + for i := 0; i < len(repositoryIDs); i++ { + id := repositoryIDs[i] + ver := versions[i] + + query += fmt.Sprintf(` + ( + SELECT crc.content_id + FROM core_repositorycontent crc + INNER JOIN core_repositoryversion crv ON (crc.version_added_id = crv.pulp_id) + LEFT OUTER JOIN core_repositoryversion crv2 ON (crc.version_removed_id = crv2.pulp_id) + WHERE crv.repository_id = '%v' AND crv.number <= %v AND NOT (crv2.number <= %v AND crv2.number IS NOT NULL) + AND rp.name ILIKE CONCAT( '%%', '%v'::text, '%%') + ) + `, id, ver, ver, filterOpts.Name) + + if i == len(repositoryIDs)-1 { + query += fmt.Sprintf(") ORDER BY rp.name ASC, rp.version ASC, rp.release ASC, rp.arch ASC LIMIT %v OFFSET %v;", pageOpts.Limit, pageOpts.Offset) + break + } + + query += "UNION" + } + rows, err := conn.Query(context.Background(), query) + if err != nil { + return nil, err + } + rpms, err := pgx.CollectRows(rows, pgx.RowToStructByName[RpmListItem]) + if err != nil { + return nil, err + } + return rpms, nil +} + func parseRepositoryVersionHrefs(hrefs []string) (repositoryIDs []string, versions []int, err error) { // /api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/ for _, href := range hrefs { From ac09116450936bf4b5cffd75c865ed45c5522272 Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Tue, 27 Feb 2024 16:34:24 -0500 Subject: [PATCH 2/7] Use prepared statements --- internal/test/integration/rpm_test.go | 31 ++++----- pkg/tangy/interface.go | 2 +- pkg/tangy/queries.go | 26 ++++++++ pkg/tangy/rpm.go | 90 +++++++++++++++++---------- 4 files changed, 102 insertions(+), 47 deletions(-) create mode 100644 pkg/tangy/queries.go diff --git a/internal/test/integration/rpm_test.go b/internal/test/integration/rpm_test.go index 24916be..172869d 100644 --- a/internal/test/integration/rpm_test.go +++ b/internal/test/integration/rpm_test.go @@ -250,42 +250,45 @@ func (r *RpmSuite) TestRpmRepositoryVersionPackageListNameFilter() { firstVersionHref := resp.LatestVersionHref require.NotNil(r.T(), firstVersionHref) + // no filter + singleList, total, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: ""}, tangy.PageOptions{}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), singleList) + assert.Equal(r.T(), total, 4) + // exact match - singleList, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "bear"}, tangy.PageOptions{}) + singleList, total, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "bear"}, tangy.PageOptions{}) require.NoError(r.T(), err) assert.NotEmpty(r.T(), singleList) + assert.Equal(r.T(), total, 1) // partial match - singleList, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "bea"}, tangy.PageOptions{}) + singleList, total, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "bea"}, tangy.PageOptions{}) require.NoError(r.T(), err) assert.NotEmpty(r.T(), singleList) + assert.Equal(r.T(), total, 1) // no match - singleList, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "wal"}, tangy.PageOptions{}) + singleList, total, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{*firstVersionHref}, tangy.RpmListFilters{Name: "wal"}, tangy.PageOptions{}) require.NoError(r.T(), err) assert.Empty(r.T(), singleList) + assert.Equal(r.T(), total, 0) } // RpmRepositoryVersionPackageList func (r *RpmSuite) TestRpmRepositoryVersionPackageListNoDuplicates() { firstVersionHref := r.firstVersionHref + secondVersionHref := r.secondVersionHref - otherRepoName := "Same Repo url " - r.CreateTestRepository(r.T(), otherRepoName) - resp, err := r.client.GetRpmRepositoryByName(r.domainName, otherRepoName) - require.NoError(r.T(), err) - secondVersionHref := resp.LatestVersionHref - require.NotNil(r.T(), firstVersionHref) - - doubleList, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, *secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) + doubleList, total, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) require.NoError(r.T(), err) assert.NotEmpty(r.T(), doubleList) + assert.Equal(r.T(), total, 4) - singleList, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) + singleList, total, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) require.NoError(r.T(), err) assert.NotEmpty(r.T(), singleList) - - assert.Equal(r.T(), singleList, doubleList) + assert.Equal(r.T(), total, 3) } func RandStringBytes(n int) string { diff --git a/pkg/tangy/interface.go b/pkg/tangy/interface.go index ecaeb55..ff8a76b 100644 --- a/pkg/tangy/interface.go +++ b/pkg/tangy/interface.go @@ -55,7 +55,7 @@ type Tangy interface { RpmRepositoryVersionPackageSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmPackageSearch, error) RpmRepositoryVersionPackageGroupSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmPackageGroupSearch, error) RpmRepositoryVersionEnvironmentSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmEnvironmentSearch, error) - RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, error) + RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, int, error) Close() } diff --git a/pkg/tangy/queries.go b/pkg/tangy/queries.go new file mode 100644 index 0000000..19f53fc --- /dev/null +++ b/pkg/tangy/queries.go @@ -0,0 +1,26 @@ +package tangy + +import ( + "fmt" + "strings" +) + +func contentIdsInVersion(repoId string, versionNum int) string { + query := ` + SELECT crc.content_id + FROM core_repositorycontent crc + INNER JOIN core_repositoryversion crv ON (crc.version_added_id = crv.pulp_id) + LEFT OUTER JOIN core_repositoryversion crv2 ON (crc.version_removed_id = crv2.pulp_id) + WHERE crv.repository_id = '%v' AND crv.number <= %v AND NOT (crv2.number <= %v AND crv2.number IS NOT NULL) + ` + return fmt.Sprintf(query, repoId, versionNum, versionNum) +} + +func contentIdsInVersions(repoVerMap map[string]int) string { + queries := []string{} + for repo, ver := range repoVerMap { + queries = append(queries, contentIdsInVersion(repo, ver)) + } + return fmt.Sprintf("( %v ) ", strings.Join(queries, " UNION ")) + +} diff --git a/pkg/tangy/rpm.go b/pkg/tangy/rpm.go index 9338f55..d09f45e 100644 --- a/pkg/tangy/rpm.go +++ b/pkg/tangy/rpm.go @@ -48,8 +48,8 @@ type RpmListItem struct { } type PageOptions struct { - Offset int32 - Limit int32 + Offset int + Limit int } type RpmListFilters struct { @@ -230,15 +230,25 @@ func buildSearchQuery(queryFragment string, search string, limit int, repository return query } +type Total struct { + Total int +} + // RpmRepositoryVersionPackageSearch search for RPMs, by name, associated to repository hrefs, returning an amount up to limit -func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, error) { +func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, int, error) { + var ( + innerUnion string + queryOpen string + countQueryOpen string + ) + if len(hrefs) == 0 { - return []RpmListItem{}, nil + return []RpmListItem{}, 0, nil } conn, err := t.pool.Acquire(ctx) if err != nil { - return nil, err + return nil, 0, err } defer conn.Release() @@ -246,44 +256,34 @@ func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs [ pageOpts.Limit = DefaultLimit } - repositoryIDs, versions, err := parseRepositoryVersionHrefs(hrefs) + repoVerMap, err := parseRepositoryVersionHrefsMap(hrefs) if err != nil { - return nil, fmt.Errorf("error parsing repository version hrefs: %w", err) + return nil, 0, fmt.Errorf("error parsing repository version hrefs: %w", err) } - query := `SELECT rp.content_ptr_id as id, rp.name, rp.version, rp.arch, rp.release, rp.epoch, rp.summary - FROM rpm_package rp WHERE rp.content_ptr_id IN (` - for i := 0; i < len(repositoryIDs); i++ { - id := repositoryIDs[i] - ver := versions[i] + countQueryOpen = "select count(*) as total FROM rpm_package rp WHERE rp.content_ptr_id IN " + innerUnion = contentIdsInVersions(repoVerMap) - query += fmt.Sprintf(` - ( - SELECT crc.content_id - FROM core_repositorycontent crc - INNER JOIN core_repositoryversion crv ON (crc.version_added_id = crv.pulp_id) - LEFT OUTER JOIN core_repositoryversion crv2 ON (crc.version_removed_id = crv2.pulp_id) - WHERE crv.repository_id = '%v' AND crv.number <= %v AND NOT (crv2.number <= %v AND crv2.number IS NOT NULL) - AND rp.name ILIKE CONCAT( '%%', '%v'::text, '%%') - ) - `, id, ver, ver, filterOpts.Name) + var countTotal int + err = conn.QueryRow(ctx, countQueryOpen+innerUnion+" AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%')", pgx.NamedArgs{"nameFilter": "%" + filterOpts.Name + "%"}).Scan(&countTotal) + if err != nil { + return nil, 0, err + } - if i == len(repositoryIDs)-1 { - query += fmt.Sprintf(") ORDER BY rp.name ASC, rp.version ASC, rp.release ASC, rp.arch ASC LIMIT %v OFFSET %v;", pageOpts.Limit, pageOpts.Offset) - break - } + queryOpen = `SELECT rp.content_ptr_id as id, rp.name, rp.version, rp.arch, rp.release, rp.epoch, rp.summary + FROM rpm_package rp WHERE rp.content_ptr_id IN ` - query += "UNION" - } - rows, err := conn.Query(context.Background(), query) + rows, err := conn.Query(ctx, queryOpen+innerUnion+ + " AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%') ORDER BY rp.name ASC, rp.version ASC, rp.release ASC, rp.arch ASC LIMIT @limit OFFSET @offset", + pgx.NamedArgs{"nameFilter": filterOpts.Name, "limit": pageOpts.Limit, "offset": pageOpts.Offset}) if err != nil { - return nil, err + return nil, 0, err } rpms, err := pgx.CollectRows(rows, pgx.RowToStructByName[RpmListItem]) if err != nil { - return nil, err + return nil, 0, err } - return rpms, nil + return rpms, countTotal, nil } func parseRepositoryVersionHrefs(hrefs []string) (repositoryIDs []string, versions []int, err error) { @@ -312,6 +312,32 @@ func parseRepositoryVersionHrefs(hrefs []string) (repositoryIDs []string, versio return } +func parseRepositoryVersionHrefsMap(hrefs []string) (mapping map[string]int, err error) { + mapping = make(map[string]int) + // /api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/ + for _, href := range hrefs { + splitHref := strings.Split(href, "/") + if len(splitHref) < 10 { + return mapping, fmt.Errorf("%v is not a valid href", splitHref) + } + id := splitHref[9] + num := splitHref[11] + + _, err = uuid.Parse(id) + if err != nil { + return mapping, fmt.Errorf("%v is not a valid uuid", id) + } + + ver, err := strconv.Atoi(num) + if err != nil { + return mapping, fmt.Errorf("%v is not a valid integer", num) + } + + mapping[id] = ver + } + return mapping, nil +} + func parsePackages(pulpPackageList []map[string]any) ([]string, error) { var packageList []string for _, pkg := range pulpPackageList { From 9157b89f86f5c658fc9ec27001a002247ff09da4 Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Wed, 28 Feb 2024 09:13:48 -0500 Subject: [PATCH 3/7] switch to embeded named params --- pkg/tangy/queries.go | 18 +++++++++++++----- pkg/tangy/rpm.go | 19 ++++++++----------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/pkg/tangy/queries.go b/pkg/tangy/queries.go index 19f53fc..befc883 100644 --- a/pkg/tangy/queries.go +++ b/pkg/tangy/queries.go @@ -3,23 +3,31 @@ package tangy import ( "fmt" "strings" + + "github.com/jackc/pgx/v5" + "golang.org/x/exp/rand" ) -func contentIdsInVersion(repoId string, versionNum int) string { +func contentIdsInVersion(repoId string, versionNum int, namedArgs *pgx.NamedArgs) string { + ran := rand.Int() + repoIdName := fmt.Sprintf("%v%v", "repoName", ran) + versionNumName := fmt.Sprintf("%v%v", "versionNum", ran) query := ` SELECT crc.content_id FROM core_repositorycontent crc INNER JOIN core_repositoryversion crv ON (crc.version_added_id = crv.pulp_id) LEFT OUTER JOIN core_repositoryversion crv2 ON (crc.version_removed_id = crv2.pulp_id) - WHERE crv.repository_id = '%v' AND crv.number <= %v AND NOT (crv2.number <= %v AND crv2.number IS NOT NULL) + WHERE crv.repository_id = @%v AND crv.number <= @%v AND NOT (crv2.number <= @%v AND crv2.number IS NOT NULL) ` - return fmt.Sprintf(query, repoId, versionNum, versionNum) + (*namedArgs)[repoIdName] = repoId + (*namedArgs)[versionNumName] = versionNum + return fmt.Sprintf(query, repoIdName, versionNumName, versionNumName) } -func contentIdsInVersions(repoVerMap map[string]int) string { +func contentIdsInVersions(repoVerMap map[string]int, namedArgs *pgx.NamedArgs) string { queries := []string{} for repo, ver := range repoVerMap { - queries = append(queries, contentIdsInVersion(repo, ver)) + queries = append(queries, contentIdsInVersion(repo, ver, namedArgs)) } return fmt.Sprintf("( %v ) ", strings.Join(queries, " UNION ")) diff --git a/pkg/tangy/rpm.go b/pkg/tangy/rpm.go index d09f45e..6a4ce45 100644 --- a/pkg/tangy/rpm.go +++ b/pkg/tangy/rpm.go @@ -236,12 +236,6 @@ type Total struct { // RpmRepositoryVersionPackageSearch search for RPMs, by name, associated to repository hrefs, returning an amount up to limit func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, int, error) { - var ( - innerUnion string - queryOpen string - countQueryOpen string - ) - if len(hrefs) == 0 { return []RpmListItem{}, 0, nil } @@ -261,21 +255,24 @@ func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs [ return nil, 0, fmt.Errorf("error parsing repository version hrefs: %w", err) } - countQueryOpen = "select count(*) as total FROM rpm_package rp WHERE rp.content_ptr_id IN " - innerUnion = contentIdsInVersions(repoVerMap) + countQueryOpen := "select count(*) as total FROM rpm_package rp WHERE rp.content_ptr_id IN " + args := pgx.NamedArgs{"nameFilter": "%" + filterOpts.Name + "%"} + innerUnion := contentIdsInVersions(repoVerMap, &args) var countTotal int - err = conn.QueryRow(ctx, countQueryOpen+innerUnion+" AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%')", pgx.NamedArgs{"nameFilter": "%" + filterOpts.Name + "%"}).Scan(&countTotal) + err = conn.QueryRow(ctx, countQueryOpen+innerUnion+" AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%')", args).Scan(&countTotal) if err != nil { return nil, 0, err } - queryOpen = `SELECT rp.content_ptr_id as id, rp.name, rp.version, rp.arch, rp.release, rp.epoch, rp.summary + queryOpen := `SELECT rp.content_ptr_id as id, rp.name, rp.version, rp.arch, rp.release, rp.epoch, rp.summary FROM rpm_package rp WHERE rp.content_ptr_id IN ` + args["limit"] = pageOpts.Limit + args["offset"] = pageOpts.Offset rows, err := conn.Query(ctx, queryOpen+innerUnion+ " AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%') ORDER BY rp.name ASC, rp.version ASC, rp.release ASC, rp.arch ASC LIMIT @limit OFFSET @offset", - pgx.NamedArgs{"nameFilter": filterOpts.Name, "limit": pageOpts.Limit, "offset": pageOpts.Offset}) + args) if err != nil { return nil, 0, err } From 405141c644246ba1ca753a816a0ea10ee81ddf22 Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Wed, 28 Feb 2024 12:42:09 -0500 Subject: [PATCH 4/7] move existing apis to new query --- internal/test/integration/rpm_test.go | 2 +- pkg/tangy/queries.go | 7 +- pkg/tangy/rpm.go | 116 ++++++++++---------------- 3 files changed, 46 insertions(+), 79 deletions(-) diff --git a/internal/test/integration/rpm_test.go b/internal/test/integration/rpm_test.go index 172869d..f04e38f 100644 --- a/internal/test/integration/rpm_test.go +++ b/internal/test/integration/rpm_test.go @@ -283,7 +283,7 @@ func (r *RpmSuite) TestRpmRepositoryVersionPackageListNoDuplicates() { doubleList, total, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) require.NoError(r.T(), err) assert.NotEmpty(r.T(), doubleList) - assert.Equal(r.T(), total, 4) + assert.Equal(r.T(), total, 5) singleList, total, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{}) require.NoError(r.T(), err) diff --git a/pkg/tangy/queries.go b/pkg/tangy/queries.go index befc883..d2e8cbc 100644 --- a/pkg/tangy/queries.go +++ b/pkg/tangy/queries.go @@ -24,11 +24,10 @@ func contentIdsInVersion(repoId string, versionNum int, namedArgs *pgx.NamedArgs return fmt.Sprintf(query, repoIdName, versionNumName, versionNumName) } -func contentIdsInVersions(repoVerMap map[string]int, namedArgs *pgx.NamedArgs) string { +func contentIdsInVersions(repoVerMap []ParsedRepoVersion, namedArgs *pgx.NamedArgs) string { queries := []string{} - for repo, ver := range repoVerMap { - queries = append(queries, contentIdsInVersion(repo, ver, namedArgs)) + for _, parsed := range repoVerMap { + queries = append(queries, contentIdsInVersion(parsed.RepositoryUUID, parsed.Version, namedArgs)) } return fmt.Sprintf("( %v ) ", strings.Join(queries, " UNION ")) - } diff --git a/pkg/tangy/rpm.go b/pkg/tangy/rpm.go index 6a4ce45..8945d5e 100644 --- a/pkg/tangy/rpm.go +++ b/pkg/tangy/rpm.go @@ -72,17 +72,18 @@ func (t *tangyImpl) RpmRepositoryVersionPackageSearch(ctx context.Context, hrefs limit = DefaultLimit } - repositoryIDs, versions, err := parseRepositoryVersionHrefs(hrefs) + repoVerMap, err := parseRepositoryVersionHrefsMap(hrefs) if err != nil { - return nil, fmt.Errorf("error parsing repository version hrefs: %w", err) + return []RpmPackageSearch{}, fmt.Errorf("error parsing repository version hrefs: %w", err) } - query := `SELECT DISTINCT ON (rp.name) rp.name, rp.summary - FROM rpm_package rp WHERE rp.content_ptr_id IN (` + args := pgx.NamedArgs{"nameFilter": "%" + search + "%", "limit": limit} + innerUnion := contentIdsInVersions(repoVerMap, &args) - query = buildSearchQuery(query, search, limit, repositoryIDs, versions) + query := `SELECT DISTINCT ON (rp.name) rp.name, rp.summary + FROM rpm_package rp WHERE rp.content_ptr_id IN ` - rows, err := conn.Query(context.Background(), query) + rows, err := conn.Query(context.Background(), query+innerUnion+" AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%') ORDER BY rp.name LIMIT @limit", args) if err != nil { return nil, err } @@ -110,17 +111,19 @@ func (t *tangyImpl) RpmRepositoryVersionPackageGroupSearch(ctx context.Context, limit = DefaultLimit } - repositoryIDs, versions, err := parseRepositoryVersionHrefs(hrefs) + repoVerMap, err := parseRepositoryVersionHrefsMap(hrefs) if err != nil { - return nil, fmt.Errorf("error parsing repository version hrefs: %w", err) + return []RpmPackageGroupSearch{}, fmt.Errorf("error parsing repository version hrefs: %w", err) } + args := pgx.NamedArgs{"nameFilter": "%" + search + "%"} + innerUnion := contentIdsInVersions(repoVerMap, &args) + query := `SELECT DISTINCT ON (rp.name, rp.id, rp.packages) rp.name, rp.id, rp.description, rp.packages - FROM rpm_packagegroup rp WHERE rp.content_ptr_id IN ( + FROM rpm_packagegroup rp WHERE rp.content_ptr_id IN ` - query = buildSearchQuery(query, search, limit, repositoryIDs, versions) - rows, err := conn.Query(context.Background(), query) + rows, err := conn.Query(ctx, query+innerUnion+"AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%') ORDER BY rp.name", args) if err != nil { return nil, err } @@ -157,11 +160,20 @@ func (t *tangyImpl) RpmRepositoryVersionPackageGroupSearch(ctx context.Context, } var searchResult []RpmPackageGroupSearch - for _, pkgGroup := range pkgGroupMap { - searchResult = append(searchResult, pkgGroup) + for _, rpm := range rpms { + nameId := rpm.Name + rpm.ID + val, ok := pkgGroupMap[nameId] + if ok { + searchResult = append(searchResult, val) + } + delete(pkgGroupMap, nameId) // delete it so we don't add it again } - return searchResult, nil + if len(searchResult) <= limit { + return searchResult, nil + } else { + return searchResult[0:limit], nil + } } // RpmRepositoryVersionEnvironmentSearch search for RPM Environments, by name, associated to repository hrefs, returning an amount up to limit @@ -180,17 +192,19 @@ func (t *tangyImpl) RpmRepositoryVersionEnvironmentSearch(ctx context.Context, h limit = DefaultLimit } - repositoryIDs, versions, err := parseRepositoryVersionHrefs(hrefs) + repoVerMap, err := parseRepositoryVersionHrefsMap(hrefs) if err != nil { - return nil, fmt.Errorf("error parsing repository version hrefs: %w", err) + return []RpmEnvironmentSearch{}, fmt.Errorf("error parsing repository version hrefs: %w", err) } + args := pgx.NamedArgs{"nameFilter": "%" + search + "%", "limit": limit} + innerUnion := contentIdsInVersions(repoVerMap, &args) + query := `SELECT DISTINCT ON (rp.name, rp.id) rp.name, rp.id, rp.description - FROM rpm_packageenvironment rp WHERE rp.content_ptr_id IN ( + FROM rpm_packageenvironment rp WHERE rp.content_ptr_id IN ` - query = buildSearchQuery(query, search, limit, repositoryIDs, versions) - rows, err := conn.Query(context.Background(), query) + rows, err := conn.Query(ctx, query+innerUnion+" AND rp.name ILIKE CONCAT( '%', @nameFilter::text, '%') ORDER BY rp.name LIMIT @limit", args) if err != nil { return nil, err } @@ -202,34 +216,6 @@ func (t *tangyImpl) RpmRepositoryVersionEnvironmentSearch(ctx context.Context, h return rpms, nil } -// buildSearchQuery builds search query for rpm package, package group, and environment search by name -func buildSearchQuery(queryFragment string, search string, limit int, repositoryIDs []string, versions []int) string { - query := queryFragment - for i := 0; i < len(repositoryIDs); i++ { - id := repositoryIDs[i] - ver := versions[i] - - query += fmt.Sprintf(` - ( - SELECT crc.content_id - FROM core_repositorycontent crc - INNER JOIN core_repositoryversion crv ON (crc.version_added_id = crv.pulp_id) - LEFT OUTER JOIN core_repositoryversion crv2 ON (crc.version_removed_id = crv2.pulp_id) - WHERE crv.repository_id = '%v' AND crv.number <= %v AND NOT (crv2.number <= %v AND crv2.number IS NOT NULL) - AND rp.name ILIKE CONCAT( '%%', '%v'::text, '%%') - ) - `, id, ver, ver, search) - - if i == len(repositoryIDs)-1 { - query += fmt.Sprintf(") ORDER BY rp.name ASC LIMIT %v;", limit) - break - } - - query += "UNION" - } - return query -} - type Total struct { Total int } @@ -283,34 +269,13 @@ func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs [ return rpms, countTotal, nil } -func parseRepositoryVersionHrefs(hrefs []string) (repositoryIDs []string, versions []int, err error) { - // /api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/ - for _, href := range hrefs { - splitHref := strings.Split(href, "/") - if len(splitHref) < 10 { - return nil, nil, fmt.Errorf("%v is not a valid href", splitHref) - } - id := splitHref[9] - num := splitHref[11] - - _, err = uuid.Parse(id) - if err != nil { - return nil, nil, fmt.Errorf("%v is not a valid uuid", id) - } - - ver, err := strconv.Atoi(num) - if err != nil { - return nil, nil, fmt.Errorf("%v is not a valid integer", num) - } - - repositoryIDs = append(repositoryIDs, id) - versions = append(versions, ver) - } - return +type ParsedRepoVersion struct { + RepositoryUUID string + Version int } -func parseRepositoryVersionHrefsMap(hrefs []string) (mapping map[string]int, err error) { - mapping = make(map[string]int) +func parseRepositoryVersionHrefsMap(hrefs []string) (mapping []ParsedRepoVersion, err error) { + mapping = []ParsedRepoVersion{} // /api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/ for _, href := range hrefs { splitHref := strings.Split(href, "/") @@ -330,7 +295,10 @@ func parseRepositoryVersionHrefsMap(hrefs []string) (mapping map[string]int, err return mapping, fmt.Errorf("%v is not a valid integer", num) } - mapping[id] = ver + mapping = append(mapping, ParsedRepoVersion{ + RepositoryUUID: id, + Version: ver, + }) } return mapping, nil } From 094af106ca3291bed4d7546d98d7864cfa91a4c4 Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Wed, 28 Feb 2024 12:55:12 -0500 Subject: [PATCH 5/7] remove struct --- pkg/tangy/rpm.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/tangy/rpm.go b/pkg/tangy/rpm.go index 8945d5e..f43c4db 100644 --- a/pkg/tangy/rpm.go +++ b/pkg/tangy/rpm.go @@ -216,10 +216,6 @@ func (t *tangyImpl) RpmRepositoryVersionEnvironmentSearch(ctx context.Context, h return rpms, nil } -type Total struct { - Total int -} - // RpmRepositoryVersionPackageSearch search for RPMs, by name, associated to repository hrefs, returning an amount up to limit func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, int, error) { if len(hrefs) == 0 { From f801b400aeef6293ff30d42bf0628c826970a75b Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Wed, 28 Feb 2024 13:06:45 -0500 Subject: [PATCH 6/7] add mock --- pkg/tangy/tangy_mock.go | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/tangy/tangy_mock.go b/pkg/tangy/tangy_mock.go index c83d006..8b3149a 100644 --- a/pkg/tangy/tangy_mock.go +++ b/pkg/tangy/tangy_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.32.0. DO NOT EDIT. +// Code generated by mockery v2.36.1. DO NOT EDIT. package tangy @@ -70,6 +70,39 @@ func (_m *MockTangy) RpmRepositoryVersionPackageGroupSearch(ctx context.Context, return r0, r1 } +// RpmRepositoryVersionPackageList provides a mock function with given fields: ctx, hrefs, filterOpts, pageOpts +func (_m *MockTangy) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, int, error) { + ret := _m.Called(ctx, hrefs, filterOpts, pageOpts) + + var r0 []RpmListItem + var r1 int + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, []string, RpmListFilters, PageOptions) ([]RpmListItem, int, error)); ok { + return rf(ctx, hrefs, filterOpts, pageOpts) + } + if rf, ok := ret.Get(0).(func(context.Context, []string, RpmListFilters, PageOptions) []RpmListItem); ok { + r0 = rf(ctx, hrefs, filterOpts, pageOpts) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]RpmListItem) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, []string, RpmListFilters, PageOptions) int); ok { + r1 = rf(ctx, hrefs, filterOpts, pageOpts) + } else { + r1 = ret.Get(1).(int) + } + + if rf, ok := ret.Get(2).(func(context.Context, []string, RpmListFilters, PageOptions) error); ok { + r2 = rf(ctx, hrefs, filterOpts, pageOpts) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + // RpmRepositoryVersionPackageSearch provides a mock function with given fields: ctx, hrefs, search, limit func (_m *MockTangy) RpmRepositoryVersionPackageSearch(ctx context.Context, hrefs []string, search string, limit int) ([]RpmPackageSearch, error) { ret := _m.Called(ctx, hrefs, search, limit) From 53a0565ebb33f3fc025d3b883246646584381b60 Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Wed, 6 Mar 2024 14:02:25 -0500 Subject: [PATCH 7/7] address feedback --- README.md | 13 ++++++++++--- internal/test/integration/rpm_test.go | 23 +++++++++++++++++++++++ pkg/tangy/queries.go | 9 +++++++++ pkg/tangy/rpm.go | 2 +- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 78a49a9..cd680b7 100644 --- a/README.md +++ b/README.md @@ -26,25 +26,32 @@ if err != nil { } defer t.Close() +// Use Tangy to list RPMs with pagination for one or more repository versions, with name filtering +versionHref := "/api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/" +rows, err := t.r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{versionHref}, tangy.RpmListFilters{Name: "kernel"}, tangy.PageOptions{Offset: 100, Limit: 20}) +if err != nil { + return err +} + // Use Tangy to search for RPMs, by name, that are associated to a specific repository version, returning up to the first 100 results versionHref := "/api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/" rows, err := t.RpmRepositoryVersionPackageSearch(context.Background(), []string{versionHref}, "bear", 100) if err != nil { -return err + return err } // Use Tangy to search for RPM Package Groups, by name, that are associated to a specific repository version, returning up to the first 100 results versionHref := "/api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/" rows, err := t.RpmRepositoryVersionPackageGroupSearch(context.Background(), []string{versionHref}, "mammals", 100) if err != nil { -return err + return err } // Use Tangy to search for RPM Environments, by name, that are associated to a specific repository version, returning up to the first 100 results versionHref := "/api/pulp/e1c6bee3/api/v3/repositories/rpm/rpm/018c1c95-4281-76eb-b277-842cbad524f4/versions/1/" rows, err := t.RpmRepositoryVersionPackageGroupSearch(context.Background(), []string{versionHref}, "animals", 100) if err != nil { -return err + return err } ``` See example.go for a complete example. diff --git a/internal/test/integration/rpm_test.go b/internal/test/integration/rpm_test.go index f04e38f..b9a004d 100644 --- a/internal/test/integration/rpm_test.go +++ b/internal/test/integration/rpm_test.go @@ -291,6 +291,29 @@ func (r *RpmSuite) TestRpmRepositoryVersionPackageListNoDuplicates() { assert.Equal(r.T(), total, 3) } +func (r *RpmSuite) TestRpmRepositoryVersionPackageListOffsetLimit() { + firstVersionHref := r.firstVersionHref + secondVersionHref := r.secondVersionHref + + list, total, err := r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{Offset: 1, Limit: 4}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), list) + assert.Equal(r.T(), 4, len(list)) + assert.Equal(r.T(), 5, total) + + list, total, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{Offset: 4, Limit: 1}) + require.NoError(r.T(), err) + assert.NotEmpty(r.T(), list) + assert.Equal(r.T(), 1, len(list)) + assert.Equal(r.T(), 5, total) + + list, total, err = r.tangy.RpmRepositoryVersionPackageList(context.Background(), []string{firstVersionHref, secondVersionHref}, tangy.RpmListFilters{}, tangy.PageOptions{Offset: 100, Limit: 100}) + require.NoError(r.T(), err) + assert.Empty(r.T(), list) + assert.Equal(r.T(), 0, len(list)) + assert.Equal(r.T(), 5, total) +} + func RandStringBytes(n int) string { const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" b := make([]byte, n) diff --git a/pkg/tangy/queries.go b/pkg/tangy/queries.go index d2e8cbc..ddb7633 100644 --- a/pkg/tangy/queries.go +++ b/pkg/tangy/queries.go @@ -8,6 +8,11 @@ import ( "golang.org/x/exp/rand" ) +// contentIdsInVersion forms a single query to fetch a list of content ids in a repository version +// +// It uses randomized query parameter names and modifies the passed in namedArgs to include the key/values for these named query parameters. +// By using randomized query parameter names, this query can be included multiple times with different repository +// versions as multiple subqueries. func contentIdsInVersion(repoId string, versionNum int, namedArgs *pgx.NamedArgs) string { ran := rand.Int() repoIdName := fmt.Sprintf("%v%v", "repoName", ran) @@ -24,6 +29,10 @@ func contentIdsInVersion(repoId string, versionNum int, namedArgs *pgx.NamedArgs return fmt.Sprintf(query, repoIdName, versionNumName, versionNumName) } +// Creates a sub query (including parenthesis) to lookup the content IDs of a list of repository versions. +// +// Takes in a pointer to Named args in order to add required named arguments for the query. Multiple queries are created +// and UNION'd together func contentIdsInVersions(repoVerMap []ParsedRepoVersion, namedArgs *pgx.NamedArgs) string { queries := []string{} for _, parsed := range repoVerMap { diff --git a/pkg/tangy/rpm.go b/pkg/tangy/rpm.go index f43c4db..e0da92b 100644 --- a/pkg/tangy/rpm.go +++ b/pkg/tangy/rpm.go @@ -216,7 +216,7 @@ func (t *tangyImpl) RpmRepositoryVersionEnvironmentSearch(ctx context.Context, h return rpms, nil } -// RpmRepositoryVersionPackageSearch search for RPMs, by name, associated to repository hrefs, returning an amount up to limit +// RpmRepositoryVersionPackageList List RPMs within a repository version, with pagination, and an optional name filter func (t *tangyImpl) RpmRepositoryVersionPackageList(ctx context.Context, hrefs []string, filterOpts RpmListFilters, pageOpts PageOptions) ([]RpmListItem, int, error) { if len(hrefs) == 0 { return []RpmListItem{}, 0, nil