Skip to content

Commit 7a4cd02

Browse files
authored
Merge branch 'fix-recommend-removed' of 'https://github.com/jjmerchante/grimoirelab-sortinghat'
Merges #1006 Closes #1006
2 parents 78d6bfc + a2a7718 commit 7a4cd02

File tree

3 files changed

+68
-8
lines changed

3 files changed

+68
-8
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
title: Unavailable Individuals in Recommendations
3+
category: fixed
4+
author: Jose Javier Merchante <[email protected]>
5+
issue: null
6+
notes: >
7+
Recommendations now handle cases where an individual has been
8+
removed or merged, preventing errors when creating new
9+
recommendations.

sortinghat/core/jobs.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import django_rq.utils
2929
import rq
3030
import redis.exceptions
31+
32+
from django.core.exceptions import ObjectDoesNotExist
3133
from django.db import IntegrityError, transaction, connection
3234
from rq.job import Job
3335

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

318320
indiv_1, indiv_2 = rec.mk, match
319321

320-
# Generate the recommendations sorting uuids alphabetical
321-
if indiv_1 == indiv_2:
322-
continue
323-
elif indiv_1 > indiv_2:
324-
indiv_1, indiv_2 = indiv_2, indiv_1
325-
326322
try:
327323
with transaction.atomic():
328324
MergeRecommendation.objects.create(individual1_id=indiv_1, individual2_id=indiv_2)
329-
except IntegrityError:
325+
except (IntegrityError, ObjectDoesNotExist):
330326
pass
331327

332328
trxl.close()

tests/test_jobs.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
AffiliationRecommendation,
5353
MergeRecommendation,
5454
GenderRecommendation)
55+
from sortinghat.core.recommendations import RecommendationEngine
5556

5657
JOB_NOT_FOUND_ERROR = "DEF not found in the registry"
5758

@@ -1243,6 +1244,60 @@ def test_not_found_uuid_error(self, mock_find_indv):
12431244

12441245
self.assertDictEqual(result, expected)
12451246

1247+
@unittest.mock.patch('sortinghat.core.jobs.RecommendationEngine')
1248+
def test_recommend_matches_with_concurrent_removal(self, mock_recommendation_engine):
1249+
"""Check if recommendations are obtained when an identity is removed while the job is running"""
1250+
1251+
ctx = SortingHatContext(self.user)
1252+
1253+
# Mock RecommendationEngine class to return a non-existing key and an existing one
1254+
def mock_recommend_matches(*args, **kwargs):
1255+
yield (self.john_smith.individual.mk,
1256+
self.john_smith.individual.mk,
1257+
['non_existing_mk', self.jsmith.individual.mk])
1258+
1259+
class MockRecommendationEngine(RecommendationEngine):
1260+
RECOMMENDATION_TYPES = {
1261+
'matches': mock_recommend_matches,
1262+
}
1263+
1264+
mock_recommendation_engine.return_value = MockRecommendationEngine()
1265+
1266+
# Test
1267+
expected = {
1268+
'results': {
1269+
self.john_smith.uuid: sorted(['non_existing_mk', self.jsm3.individual.mk])
1270+
}
1271+
}
1272+
recommendations_expected = [
1273+
sorted([self.jsmith.individual.mk, self.john_smith.individual.mk])
1274+
]
1275+
1276+
source_uuids = [self.john_smith.uuid]
1277+
target_uuids = [self.john_smith.uuid,
1278+
self.jsmith.uuid]
1279+
1280+
criteria = ['email', 'name', 'username']
1281+
1282+
job = recommend_matches.delay(ctx,
1283+
source_uuids,
1284+
target_uuids,
1285+
criteria)
1286+
result = job.result
1287+
1288+
# Preserve job results order for the comparison against the expected results
1289+
for key in result['results']:
1290+
result['results'][key] = sorted(result['results'][key])
1291+
1292+
self.assertDictEqual(result, expected)
1293+
1294+
self.assertEqual(MergeRecommendation.objects.count(), 1)
1295+
1296+
for rec in recommendations_expected:
1297+
self.assertTrue(
1298+
MergeRecommendation.objects.filter(individual1=rec[0],
1299+
individual2=rec[1]).exists())
1300+
12461301
def test_transactions(self):
12471302
"""Check if the right transactions were created"""
12481303

0 commit comments

Comments
 (0)