Skip to content

Commit ca2e6ba

Browse files
author
Andrew Cheng
committed
Add base_version filter to compute net content diffs across arbitrary repository versions
This PR adds a base_version filter to the content viewset that lets repository_version_added and repository_version_removed compute the net content diff between two arbitray (non-adjacent) repository versions, rather than only the single-step diff against the immediate predecessor. Benchmarks on a synthetic 200k-unit system with 1000-unit diffs between repo versions show that implementing a subquery vs inlined-parameter form is ~12-13x faster, with the query going from 1471ms -> 116ms for a 50k/50k repo version pair and 6802ms -> 521ms for a 199k/199k repo version pair, qith query planning time dropping from 72ms -> 1.6ms. When base_version is omitted, behavior is unchanged (single-step diff), preserving backward compatibility. If base_version is provided without repository_version_added or repository_version_removed, nothing happens. Closes #7831 Assisted by: Claude Opus 4.8
1 parent 94f63c3 commit ca2e6ba

5 files changed

Lines changed: 157 additions & 11 deletions

File tree

CHANGES/7831.feature

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Added an optional ``base_version`` filter to the content list endpoints. When combined with
2+
``repository_version_added`` or ``repository_version_removed``, it returns the net set of content
3+
added or removed between two arbitrary repository versions instead of only the single-step
4+
difference against the filtered version's immediate predecessor.

pulpcore/app/models/repository.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,13 +1000,26 @@ def get_content(self, content_qs=None):
10001000
content_ids = self.content_ids
10011001
if len(content_ids) >= 65535:
10021002
# Workaround for PostgreSQL's limit on the number of parameters in a query
1003-
content_ids = (
1004-
RepositoryVersion.objects.filter(pk=self.pk)
1005-
.annotate(cids=Func(F("content_ids"), function="unnest"))
1006-
.values_list("cids", flat=True)
1007-
)
1003+
content_ids = self.content_ids_subquery()
10081004
return content_qs.filter(pk__in=content_ids)
10091005

1006+
def content_ids_subquery(self):
1007+
"""
1008+
Return this version's ``content_ids`` as a database-side ``unnest`` subquery.
1009+
1010+
Using a subquery keeps the content unit UUIDs inside PostgreSQL instead of loading the
1011+
whole array into Python and passing each UUID as a bound query parameter. This avoids the
1012+
per-query parameter limit and the memory/serialization cost for large repository versions.
1013+
1014+
Returns:
1015+
django.db.models.QuerySet: A values queryset yielding the content unit UUIDs.
1016+
"""
1017+
return (
1018+
RepositoryVersion.objects.filter(pk=self.pk)
1019+
.annotate(cids=Func(F("content_ids"), function="unnest"))
1020+
.values_list("cids", flat=True)
1021+
)
1022+
10101023
@property
10111024
def content(self):
10121025
"""
@@ -1119,8 +1132,8 @@ def added(self, base_version=None):
11191132
if not base_version:
11201133
return Content.objects.filter(version_memberships__version_added=self)
11211134

1122-
return Content.objects.filter(pk__in=self.content_ids).exclude(
1123-
pk__in=base_version.content_ids
1135+
return Content.objects.filter(pk__in=self.content_ids_subquery()).exclude(
1136+
pk__in=base_version.content_ids_subquery()
11241137
)
11251138

11261139
def removed(self, base_version=None):
@@ -1134,8 +1147,8 @@ def removed(self, base_version=None):
11341147
if not base_version:
11351148
return Content.objects.filter(version_memberships__version_removed=self)
11361149

1137-
return Content.objects.filter(pk__in=base_version.content_ids).exclude(
1138-
pk__in=self.content_ids
1150+
return Content.objects.filter(pk__in=base_version.content_ids_subquery()).exclude(
1151+
pk__in=self.content_ids_subquery()
11391152
)
11401153

11411154
def contains(self, content):

pulpcore/app/viewsets/content.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
ContentAddedRepositoryVersionFilter,
2424
ContentRemovedRepositoryVersionFilter,
2525
ContentRepositoryVersionFilter,
26+
RepositoryVersionBaseFilter,
2627
)
2728

2829

@@ -125,6 +126,10 @@ class ContentFilter(BaseFilterSet):
125126
Return Content which was added in this repository version.
126127
repository_version_removed:
127128
Return Content which was removed from this repository version.
129+
base_version:
130+
When combined with repository_version_added / repository_version_removed, compute the
131+
net difference relative to this base repository version instead of the filtered
132+
version's immediate predecessor. Has no effect on its own.
128133
orphaned_for:
129134
Return Content which has been orphaned for a given number of minutes;
130135
-1 uses ORPHAN_PROTECTION_TIME value.
@@ -135,6 +140,7 @@ class ContentFilter(BaseFilterSet):
135140
repository_version = ContentRepositoryVersionFilter()
136141
repository_version_added = ContentAddedRepositoryVersionFilter()
137142
repository_version_removed = ContentRemovedRepositoryVersionFilter()
143+
base_version = RepositoryVersionBaseFilter()
138144
orphaned_for = OrphanedFilter(
139145
help_text="Minutes Content has been orphaned for. -1 uses ORPHAN_PROTECTION_TIME."
140146
)

pulpcore/app/viewsets/custom_filters.py

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,53 @@ def filter(self, qs, value):
162162
"""
163163
raise NotImplementedError()
164164

165+
def get_base_version(self, field_name="base_version"):
166+
"""
167+
Resolve the companion ``base_version`` filter, if the request supplied one.
168+
169+
The added/removed filters use this to diff against an *arbitrary* base repository
170+
version instead of the filtered version's immediate predecessor.
171+
172+
Args:
173+
field_name (string): The name of the companion base-version filter on the filterset.
174+
175+
Returns:
176+
pulpcore.app.models.RepositoryVersion or None: The resolved base version, or None
177+
when no base version was supplied.
178+
"""
179+
if self.parent is None:
180+
return None
181+
base_value = self.parent.form.cleaned_data.get(field_name)
182+
if not base_value:
183+
return None
184+
return self.get_repository_version(base_value)
185+
186+
187+
class RepositoryVersionBaseFilter(RepoVersionHrefPrnFilter):
188+
"""
189+
Companion filter that designates the base repository version for
190+
``repository_version_added`` / ``repository_version_removed`` diffs.
191+
192+
On its own this filter does not alter the queryset. It is consumed by the added/removed
193+
filters to compute the net difference between two arbitrary repository versions, rather than
194+
the single-step difference against the filtered version's immediate predecessor.
195+
"""
196+
197+
def __init__(self, *args, **kwargs):
198+
kwargs.setdefault(
199+
"help_text",
200+
_(
201+
"Repository Version referenced by HREF/PRN to use as the base for "
202+
"repository_version_added / repository_version_removed. When set, added/removed "
203+
"content is computed relative to this version instead of the immediate predecessor."
204+
),
205+
)
206+
super().__init__(*args, **kwargs)
207+
208+
def filter(self, qs, value):
209+
# No-op on its own; the value is consumed by the added/removed filters.
210+
return qs
211+
165212

166213
class RepositoryVersionFilter(RepoVersionHrefPrnFilter):
167214
"""
@@ -251,7 +298,7 @@ def filter(self, qs, value):
251298
return qs
252299

253300
repo_version = self.get_repository_version(value)
254-
return qs.filter(pk__in=repo_version.added())
301+
return qs.filter(pk__in=repo_version.added(base_version=self.get_base_version()))
255302

256303

257304
class ContentRemovedRepositoryVersionFilter(RepoVersionHrefPrnFilter):
@@ -273,7 +320,7 @@ def filter(self, qs, value):
273320
return qs
274321

275322
repo_version = self.get_repository_version(value)
276-
return qs.filter(pk__in=repo_version.removed())
323+
return qs.filter(pk__in=repo_version.removed(base_version=self.get_base_version()))
277324

278325

279326
class CharInFilter(BaseInFilter, CharFilter):

pulpcore/tests/functional/api/using_plugin/test_repo_versions.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,82 @@ def test_add_remove_content(
188188
assert latest_version.content_summary.removed == {}
189189

190190

191+
@pytest.fixture
192+
def file_repo_three_versions(
193+
file_bindings,
194+
file_repository_factory,
195+
file_9_contents,
196+
monitor_task,
197+
):
198+
"""Build a repo with three versions whose diffs span more than one step.
199+
200+
Using the content units "A" through "E", the versions end up as::
201+
202+
v1: add A, B, C -> present {A, B, C}
203+
v2: add D, remove A -> present {B, C, D}
204+
v3: add E, remove B -> present {C, D, E}
205+
206+
So a diff between the non-adjacent versions v1 and v3 differs from the single-step diff at v3.
207+
"""
208+
repo = file_repository_factory()
209+
contents = file_9_contents
210+
211+
def modify(add=(), remove=()):
212+
body = {
213+
"add_content_units": [contents[name].pulp_href for name in add],
214+
"remove_content_units": [contents[name].pulp_href for name in remove],
215+
}
216+
monitor_task(file_bindings.RepositoriesFileApi.modify(repo.pulp_href, body).task)
217+
return file_bindings.RepositoriesFileApi.read(repo.pulp_href).latest_version_href
218+
219+
v1 = modify(add=["A", "B", "C"])
220+
v2 = modify(add=["D"], remove=["A"])
221+
v3 = modify(add=["E"], remove=["B"])
222+
return repo, v1, v2, v3
223+
224+
225+
def _relative_paths(list_response):
226+
return {content.relative_path for content in list_response.results}
227+
228+
229+
@pytest.mark.parallel
230+
def test_repository_version_added_base_version(file_bindings, file_repo_three_versions):
231+
"""``base_version`` turns ``repository_version_added`` into a net diff between two versions."""
232+
_, v1, v2, v3 = file_repo_three_versions
233+
234+
# Without base_version: single-step diff against the immediate predecessor (v2 -> v3).
235+
single_step = file_bindings.ContentFilesApi.list(repository_version_added=v3)
236+
assert _relative_paths(single_step) == {"E"}
237+
238+
# With base_version=v1: net content in v3 but not in v1 => {C, D, E} - {A, B, C}.
239+
net = file_bindings.ContentFilesApi.list(repository_version_added=v3, base_version=v1)
240+
assert _relative_paths(net) == {"D", "E"}
241+
242+
243+
@pytest.mark.parallel
244+
def test_repository_version_removed_base_version(file_bindings, file_repo_three_versions):
245+
"""``base_version`` turns ``repository_version_removed`` into a net diff across versions."""
246+
_, v1, v2, v3 = file_repo_three_versions
247+
248+
# Without base_version: single-step diff against the immediate predecessor (v2 -> v3).
249+
single_step = file_bindings.ContentFilesApi.list(repository_version_removed=v3)
250+
assert _relative_paths(single_step) == {"B"}
251+
252+
# With base_version=v1: net content in v1 but not in v3 => {A, B, C} - {C, D, E}.
253+
net = file_bindings.ContentFilesApi.list(repository_version_removed=v3, base_version=v1)
254+
assert _relative_paths(net) == {"A", "B"}
255+
256+
257+
@pytest.mark.parallel
258+
def test_base_version_ignored_without_added_or_removed(file_bindings, file_repo_three_versions):
259+
"""``base_version`` has no effect unless paired with added/removed filters."""
260+
_, v1, v2, v3 = file_repo_three_versions
261+
262+
present = file_bindings.ContentFilesApi.list(repository_version=v3)
263+
present_with_base = file_bindings.ContentFilesApi.list(repository_version=v3, base_version=v1)
264+
assert _relative_paths(present_with_base) == _relative_paths(present) == {"C", "D", "E"}
265+
266+
191267
@pytest.mark.parallel
192268
def test_add_remove_repo_version(
193269
file_bindings,

0 commit comments

Comments
 (0)