-
Notifications
You must be signed in to change notification settings - Fork 13
Fix/updateinfo bug fix #75
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,12 +101,10 @@ def generate_updateinfo_xml( | |
| managing_editor: Editor email for XML header | ||
| company_name: Company name for copyright | ||
| supported_product_id: Optional supported_product_id for FK-based filtering (v2) | ||
| product_name_for_packages: Optional product_name for legacy filtering (v1) | ||
| product_name_for_packages: Product_name used for legacy filtering (v1) and default_collection and naming XML elements | ||
|
|
||
| Returns: | ||
| XML string in updateinfo.xml format | ||
|
|
||
| Note: Either supported_product_id (v2) or product_name_for_packages (v1) must be provided. | ||
| """ | ||
| advisories = {} | ||
| for affected_product in affected_products: | ||
|
|
@@ -201,13 +199,14 @@ def generate_updateinfo_xml( | |
| ] | ||
|
|
||
| if supported_product_id is not None: | ||
| # v2: Filter by FK (normalized relational data) | ||
| filtered_packages = [ | ||
| pkg for pkg in advisory.packages | ||
| if pkg.supported_product_id == supported_product_id and pkg.repo_name == repo_name | ||
| ] | ||
| seen_nevras = set() | ||
| filtered_packages = [] | ||
| for pkg in advisory.packages: | ||
| if pkg.supported_product_id == supported_product_id and pkg.repo_name == repo_name: | ||
| if pkg.nevra not in seen_nevras: | ||
| seen_nevras.add(pkg.nevra) | ||
| filtered_packages.append(pkg) | ||
rockythorn marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
201
to
+208
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This dedupe implementation is clean, but I'd be remiss (i.e., exiled by ##python on Libera.chat) if I didn't take the opportunity to mention a if supported_product_id is not None:
filtered_packages = list({
pkg.nevra: pkg
for pkg in advisory.packages
if pkg.supported_product_id == supported_product_id
and pkg.repo_name == repo_name
}.values())As long as we definitely want the first match and don't need side effects or logging during the filtering, then using a dict naturally performs the Another option would be to remove the nesting and perform the check in a single pass instead: # ...
for pkg in advisory.packages:
if (
pkg.supported_product_id != supported_product_id
or pkg.repo_name != repo_name
or pkg.nevra in seen_nevras
):
continue
seen_nevras.add(pkg.nevra)
filtered_packages.append(pkg)Either way, the code is fine as is :) |
||
| else: | ||
| # v1: Filter by product_name (legacy denormalized field) | ||
| filtered_packages = [ | ||
| pkg for pkg in advisory.packages | ||
| if pkg.product_name == product_name_for_packages and pkg.repo_name == repo_name | ||
|
|
@@ -474,6 +473,7 @@ async def get_updateinfo_v2( | |
| managing_editor=managing_editor, | ||
| company_name=company_name, | ||
| supported_product_id=supported_product.id, | ||
| product_name_for_packages=f"{product_name} {major_version} {arch}", | ||
| ) | ||
|
|
||
| return Response(content=xml_str, media_type="application/xml") | ||
Uh oh!
There was an error while loading. Please reload this page.