Skip to content
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

Fixes: #18263 - Iterate through a freshly queried set of CableTerminations to find endpoints in update_connected_endpoints #18264

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

bctiemann
Copy link
Contributor

@bctiemann bctiemann commented Dec 21, 2024

Fixes: #18263

During an API PATCH update to a Cable where one or both endpoints is changed, the signal handler update_connected_endpoints iterates through self.terminations.all() which in the case of an API call has not necessarily been refreshed after the deletion of stale endpoints at

# Delete stale CableTerminations
if self._terminations_modified:
for termination, ct in a_terminations.items():
if termination.pk and termination not in self.a_terminations:
ct.delete()
for termination, ct in b_terminations.items():
if termination.pk and termination not in self.b_terminations:
ct.delete()
# Save new CableTerminations (if any)
if self._terminations_modified:
for termination in self.a_terminations:
if not termination.pk or termination not in a_terminations:
CableTermination(cable=self, cable_end='A', termination=termination).save()
for termination in self.b_terminations:
if not termination.pk or termination not in b_terminations:
CableTermination(cable=self, cable_end='B', termination=termination).save()

This leads to the local_cable_terminations being empty here (because the pks don't match):

# Step 6: Determine the far-end terminations
if isinstance(links[0], Cable):
termination_type = ObjectType.objects.get_for_model(terminations[0])
local_cable_terminations = CableTermination.objects.filter(
termination_type=termination_type,
termination_id__in=[t.pk for t in terminations]
)
q_filter = Q()
for lct in local_cable_terminations:
cable_end = 'A' if lct.cable_end == 'B' else 'B'
q_filter |= Q(cable=lct.cable, cable_end=cable_end)
remote_cable_terminations = CableTermination.objects.filter(q_filter)
remote_terminations = [ct.termination for ct in remote_cable_terminations]
(Step 6 of Cable.from_origin), which leads to q_filter being empty and thus the completely unfiltered iteration at L611 potentially taking O(n) time depending on the number of terminations in the DB.

This change ensures that the iteration of CableTerminations associated with a Cable after an update is freshly queried from the DB following the deletion of stale endpoints rather than relying on the (potentially stale or cached) self.terminations reverse relation, and thus ensuring from_origin is calculated correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cable terminations are not refreshed correctly on API PATCH of Cable
1 participant