Skip to content

Fixes #33525 - Add debs packages_restrict_latest #9671

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

Open
m-bucher wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixes #33525 - Add debs packages_restrict_latest #9671

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
9 changes: 7 additions & 2 deletions app/models/katello/deb.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,13 @@ def self.apt_installable_for_host(host)
Katello::Deb.in_repositories(repos).where.not(name: host.installed_debs.pluck(:name)).order(:name)
end

def self.latest(_relation)
fail 'NotImplemented'
def self.latest(relation)
# This might be very slow
Copy link
Member

@ianballou ianballou Sep 25, 2025

Choose a reason for hiding this comment

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

Some LLM fun to try avoiding the left outer join:

  def self.latest(relation)
    # Create a CTE to avoid double subquery execution
    sql = "WITH base_data AS (#{relation.to_sql})
           SELECT bd1.* FROM base_data bd1
           WHERE NOT EXISTS (
             SELECT 1 FROM base_data bd2 
             WHERE bd1.name = bd2.name 
             AND bd1.architecture = bd2.architecture 
             AND deb_version_cmp(bd1.version, bd2.version) < 0
           )"
    relation.from("(#{sql}) optimized_latest")
  end

return relation.joins(
"LEFT OUTER JOIN(#{relation.to_sql}) AS katello_debs2 ON " \
'katello_debs.name = katello_debs2.name AND katello_debs.architecture = katello_debs2.architecture AND ' \
'deb_version_cmp(katello_debs.version, katello_debs2.version) < 0 ' \
).where('katello_debs2.id IS NULL')
Comment on lines +108 to +112
Copy link
Member

@adamruzicka adamruzicka Feb 7, 2024

Choose a reason for hiding this comment

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

Capturing things we talked about live: If I were doing this I'd probably drop the explicit return and used a squiggly heredoc, but that's just personal preference so feel free to ignore.

Copy link
Contributor Author

@m-bucher m-bucher Feb 19, 2024

Choose a reason for hiding this comment

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

Since most of the code was shamelessly copied over from app/models/katello/rpm.rb, I would keep it that way for being consistent.

Eventually, we should think about enforcing a certain style through rubocop. AFAIK Katello's rubocop rules are currently pretty flexible.

end
end
end
10 changes: 10 additions & 0 deletions test/controllers/api/v2/debs_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ def models
@version = ContentViewVersion.first
@deb = katello_debs(:one)
@host = hosts(:one)
@org = get_organization
end

def setup
Expand Down Expand Up @@ -70,6 +71,15 @@ def test_index_with_available_for_content_view_version
assert_includes ids, @deb.id
end

def test_index_with_latest
response = get :index, params: { :packages_restrict_latest => true, :organization_id => @org.id }

assert_response :success
ids = JSON.parse(response.body)['results'].map { |p| p['id'] }
assert_includes ids, katello_debs(:one_new).id
refute_includes ids, @deb.id
end

def test_index_protected
assert_protected_action(:index, @auth_permissions, @unauth_permissions) do
get :index, params: { :repository_id => @repo.id }
Expand Down
1 change: 1 addition & 0 deletions webpack/components/extensions/HostDetails/Tabs/DebsTab/InstallableDebsActions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const getHostInstallableDebs = (hostId, params) => get({
host_id: hostId,
packages_restrict_not_installed: true,
packages_restrict_applicable: false,
packages_restrict_latest: true,
},
});
export default getHostInstallableDebs;
Expand Down