Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
title: Unavailable Individuals in Recommendations
category: fixed
author: Jose Javier Merchante <[email protected]>
issue: null
notes: >
Recommendations now handle cases where an individual has been
removed or merged, preventing errors when creating new
recommendations.
12 changes: 4 additions & 8 deletions sortinghat/core/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import django_rq.utils
import rq
import redis.exceptions

from django.core.exceptions import ObjectDoesNotExist
from django.db import IntegrityError, transaction, connection
from rq.job import Job

Expand Down Expand Up @@ -219,7 +221,7 @@ def recommend_affiliations(ctx, uuids=None, last_modified=MIN_PERIOD_DATE):
with transaction.atomic():
AffiliationRecommendation.objects.create(individual_id=rec.mk,
organization=org)
except IntegrityError:
except (IntegrityError, ObjectDoesNotExist):
logger.debug(
f"Job {job.id} 'Unable to create affiliation recommendation for"
f"Individual {rec.key} and Organization {org_name}"
Expand Down Expand Up @@ -317,16 +319,10 @@ def recommend_matches(ctx, source_uuids,

indiv_1, indiv_2 = rec.mk, match

# Generate the recommendations sorting uuids alphabetical
if indiv_1 == indiv_2:
continue
elif indiv_1 > indiv_2:
indiv_1, indiv_2 = indiv_2, indiv_1

try:
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not needed anymore?

with transaction.atomic():
MergeRecommendation.objects.create(individual1_id=indiv_1, individual2_id=indiv_2)
except IntegrityError:
except (IntegrityError, ObjectDoesNotExist):
pass

trxl.close()
Expand Down
55 changes: 55 additions & 0 deletions tests/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
AffiliationRecommendation,
MergeRecommendation,
GenderRecommendation)
from sortinghat.core.recommendations import RecommendationEngine

JOB_NOT_FOUND_ERROR = "DEF not found in the registry"

Expand Down Expand Up @@ -1243,6 +1244,60 @@ def test_not_found_uuid_error(self, mock_find_indv):

self.assertDictEqual(result, expected)

@unittest.mock.patch('sortinghat.core.jobs.RecommendationEngine')
def test_recommend_matches_with_concurrent_removal(self, mock_recommendation_engine):
"""Check if recommendations are obtained when an identity is removed while the job is running"""

ctx = SortingHatContext(self.user)

# Mock RecommendationEngine class to return a non-existing key and an existing one
def mock_recommend_matches(*args, **kwargs):
yield (self.john_smith.individual.mk,
self.john_smith.individual.mk,
['non_existing_mk', self.jsmith.individual.mk])

class MockRecommendationEngine(RecommendationEngine):
RECOMMENDATION_TYPES = {
'matches': mock_recommend_matches,
}

mock_recommendation_engine.return_value = MockRecommendationEngine()

# Test
expected = {
'results': {
self.john_smith.uuid: sorted(['non_existing_mk', self.jsm3.individual.mk])
}
}
recommendations_expected = [
sorted([self.jsmith.individual.mk, self.john_smith.individual.mk])
]

source_uuids = [self.john_smith.uuid]
target_uuids = [self.john_smith.uuid,
self.jsmith.uuid]

criteria = ['email', 'name', 'username']

job = recommend_matches.delay(ctx,
source_uuids,
target_uuids,
criteria)
result = job.result

# Preserve job results order for the comparison against the expected results
for key in result['results']:
result['results'][key] = sorted(result['results'][key])

self.assertDictEqual(result, expected)

self.assertEqual(MergeRecommendation.objects.count(), 1)

for rec in recommendations_expected:
self.assertTrue(
MergeRecommendation.objects.filter(individual1=rec[0],
individual2=rec[1]).exists())

def test_transactions(self):
"""Check if the right transactions were created"""

Expand Down