Skip to content

fix(all_urls):SP-4237 add is_mined and package_hash filter condition… #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
agustingroh wants to merge 1 commit into from
Closed

fix(all_urls):SP-4237 add is_mined and package_hash filter condition… #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.10.0] - 2026-04-06
### Fixed
- Added `is_mined != false AND package_hash != ''` filter conditions to `GetURLsByPurlNameType`, `GetURLsByPurlNameTypeVersion`, and `GetVersionsByPurlNameType` queries to exclude unmined and empty-hash records
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Changelog entry is missing one updated query method.

Line [12] documents three methods, but this PR also changes CheckPurlByNameType in pkg/models/all_urls.go (Lines [178-179]). Please include it to keep release notes accurate.

Suggested changelog tweak 
-- Added `is_mined != false AND package_hash != ''` filter conditions to `GetURLsByPurlNameType`, `GetURLsByPurlNameTypeVersion`, and `GetVersionsByPurlNameType` queries to exclude unmined and empty-hash records
+- Added `is_mined != false AND package_hash != ''` filter conditions to `GetURLsByPurlNameType`, `GetURLsByPurlNameTypeVersion`, `GetVersionsByPurlNameType`, and `CheckPurlByNameType` queries to exclude unmined and empty-hash records
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Added `is_mined != false AND package_hash != ''` filter conditions to `GetURLsByPurlNameType`, `GetURLsByPurlNameTypeVersion`, and `GetVersionsByPurlNameType` queries to exclude unmined and empty-hash records
- Added `is_mined != false AND package_hash != ''` filter conditions to `GetURLsByPurlNameType`, `GetURLsByPurlNameTypeVersion`, `GetVersionsByPurlNameType`, and `CheckPurlByNameType` queries to exclude unmined and empty-hash records
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` at line 12, Update the changelog entry to also list the
CheckPurlByNameType query as being modified: editing the line that currently
mentions GetURLsByPurlNameType, GetURLsByPurlNameTypeVersion, and
GetVersionsByPurlNameType to add CheckPurlByNameType (the function in
pkg/models/all_urls.go around the CheckPurlByNameType implementation) so the
release notes reflect that CheckPurlByNameType now also includes the `is_mined
!= false AND package_hash != ''` filter.


## [0.9.0] - 2026-04-01
### Added
- Added `GetSPDXLicenseDetails` method in `LicenseModel` to retrieve SPDX license details by ID from the `spdx_license_data` table
Expand Down Expand Up @@ -107,4 +111,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[0.6.0]: https://github.com/scanoss/go-models/compare/v0.5.1...v0.6.0
[0.7.0]: https://github.com/scanoss/go-models/compare/v0.6.0...v0.7.0
[0.8.0]: https://github.com/scanoss/go-models/compare/v0.7.0...v0.8.0
[0.9.0]: https://github.com/scanoss/go-models/compare/v0.8.0...v0.9.0
[0.9.0]: https://github.com/scanoss/go-models/compare/v0.8.0...v0.9.0
[0.10.0]: https://github.com/scanoss/go-models/compare/v0.9.0...v0.10.0
15 changes: 11 additions & 4 deletions pkg/models/all_urls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ func (m *AllUrlsModel) GetURLsByPurlNameType(ctx context.Context, purlName, purl
" LEFT JOIN mines m ON u.mine_id = m.id" +
" LEFT JOIN licenses l ON u.license_id = l.id" +
" LEFT JOIN versions v ON u.version_id = v.id" +
" WHERE m.purl_type = $1 AND u.purl_name = $2 ORDER BY date DESC"
" WHERE m.purl_type = $1 AND u.purl_name = $2" +
" AND u.is_mined != false AND u.package_hash != ''" +
Copy link
Copy Markdown

@scanoss-qg scanoss-qg Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could is_mined flag be configurable throw a parameter?

" ORDER BY date DESC"
Comment on lines +74 to +76
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Blocking: new filters break queries on current DB schema.

The added predicates on Lines [75], [115], [148], and [179] are causing runtime SQL failures (no such column: ...is_mined) per pipeline logs. This currently breaks all affected lookup paths.

Please ship this with matching schema support (migration + fixtures) or add a compatibility path for DBs that don’t yet have all_urls.is_mined / all_urls.package_hash.

Also applies to: 114-116, 147-149, 178-179

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/models/all_urls.go` around lines 74 - 76, The new WHERE predicates
referencing all_urls.is_mined and all_urls.package_hash in
pkg/models/all_urls.go break older DBs; add a compatibility check at startup
(e.g., query PRAGMA table_info('all_urls') or information_schema for your DB) to
detect presence of columns is_mined and package_hash, expose two booleans
(hasIsMined, hasPackageHash), and change the SQL builder(s) in
pkg/models/all_urls.go that currently concatenate "AND u.is_mined != false" /
"AND u.package_hash != ''" (the three query strings shown) to conditionally
append those predicates only when the corresponding boolean is true, or provide
a migration path that adds the columns and fixtures before using the new
queries.


var allUrls []AllURL
err := m.db.SelectContext(ctx, &allUrls, query, purlType, purlName)
Expand Down Expand Up @@ -109,7 +111,9 @@ func (m *AllUrlsModel) GetURLsByPurlNameTypeVersion(ctx context.Context, purlNam
" LEFT JOIN mines m ON u.mine_id = m.id" +
" LEFT JOIN licenses l ON u.license_id = l.id" +
" LEFT JOIN versions v ON u.version_id = v.id" +
" WHERE m.purl_type = $1 AND u.purl_name = $2 AND (v.version_name = $3 OR v.version_name = $4) ORDER BY date DESC"
" WHERE m.purl_type = $1 AND u.purl_name = $2 AND (v.version_name = $3 OR v.version_name = $4)" +
" AND u.is_mined != false AND u.package_hash != ''" +
" ORDER BY date DESC"

var allUrls []AllURL
err := m.db.SelectContext(ctx, &allUrls, query, purlType, purlName, purlVersion, semverV)
Expand Down Expand Up @@ -140,7 +144,9 @@ func (m *AllUrlsModel) GetVersionsByPurlNameType(ctx context.Context, purlName,
" LEFT JOIN mines m ON u.mine_id = m.id" +
" LEFT JOIN licenses l ON u.license_id = l.id" +
" LEFT JOIN versions v ON u.version_id = v.id" +
" WHERE m.purl_type = $1 AND u.purl_name = $2 ORDER BY date DESC"
" WHERE m.purl_type = $1 AND u.purl_name = $2" +
" AND u.is_mined != false AND u.package_hash != ''" +
" ORDER BY date DESC"

var allUrls []AllURL
err := m.db.SelectContext(ctx, &allUrls, query, purlType, purlName)
Expand Down Expand Up @@ -169,7 +175,8 @@ func (m *AllUrlsModel) CheckPurlByNameType(ctx context.Context, purlName string,
"SELECT EXISTS("+
"SELECT 1 FROM all_urls au"+
" INNER JOIN mines m ON au.mine_id = m.id"+
" WHERE au.purl_name = $1 AND m.purl_type = $2)",
" WHERE au.purl_name = $1 AND m.purl_type = $2"+
" AND au.is_mined != false AND au.package_hash != '')",
purlName, purlType).Scan(&exists)
if err != nil {
s.Errorf("Error: Failed to query all_urls table for %v, %v: %v", purlName, purlType, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (cs *ComponentService) GetComponentVersions(ctx context.Context, purl strin

func (cs *ComponentService) pickOneUrl(ctx context.Context, allUrls []models.AllURL, purlName, purlType, purlReq string) (models.AllURL, error) {
s := ctxzap.Extract(ctx).Sugar()

s.Debugf("Picking one URL from %v", allUrls)
if len(allUrls) == 0 {
s.Infof("No component match (in urls) found for %v, %v", purlName, purlType)
return models.AllURL{}, ErrVersionNotFound
Expand Down
Loading