Skip to content

Commit 5f4d832

Browse files
bbimberbbimber
committed
Better handle MCC complete() when prior code fails
1 parent 462731f commit 5f4d832

2 files changed

Lines changed: 65 additions & 57 deletions

File tree

‎mcc/resources/queries/study/demographics.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ function onInit(event, helper){
2020
}
2121

2222
function onUpsert(helper, scriptErrors, row, oldRow){
23+
row.objectId = row.objectId || oldRow?.objectId || LABKEY.Utils.generateUUID().toUpperCase()
24+
2325
if (row.status && row.status.match(/Undetermined/)) {
2426
row.status = 'Unknown';
2527
}

‎mcc/src/org/labkey/mcc/query/TriggerHelper.java‎

Lines changed: 63 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.labkey.api.data.ContainerManager;
1111
import org.labkey.api.data.DbSchema;
1212
import org.labkey.api.data.DbSchemaType;
13+
import org.labkey.api.data.DbScope;
1314
import org.labkey.api.data.DbSequence;
1415
import org.labkey.api.data.DbSequenceManager;
1516
import org.labkey.api.data.SimpleFilter;
@@ -29,6 +30,7 @@
2930
import org.labkey.api.security.ValidEmail;
3031
import org.labkey.api.security.permissions.DeletePermission;
3132
import org.labkey.api.settings.AppProps;
33+
import org.labkey.api.study.StudyService;
3234
import org.labkey.api.util.MailHelper;
3335
import org.labkey.api.util.PageFlowUtil;
3436
import org.labkey.api.view.UnauthorizedException;
@@ -297,74 +299,83 @@ private TableInfo getMappingTable()
297299

298300
public int ensureMccAliasExists(Collection<String> rawIds, Map<Object, Object> existingAliases)
299301
{
300-
clearCachedTables();
302+
try (DbScope.Transaction transaction = StudyService.get().getDatasetSchema().getScope().ensureTransaction())
303+
{
304+
if (transaction.isAborted())
305+
{
306+
return 0;
307+
}
301308

302-
// NOTE: The incoming object can convert numeric IDs from strings to int, so manually convert:
303-
// Also, CaseInsensitiveSet will convert the keys to lowercase, which is problematic for case-sensitive databases
304-
final CaseInsensitiveHashMap<String> idMap = new CaseInsensitiveHashMap<>();
305-
rawIds.stream().map(String::valueOf).forEach(x -> idMap.put(x, x));
309+
// NOTE: The incoming object can convert numeric IDs from strings to int, so manually convert:
310+
// Also, CaseInsensitiveSet will convert the keys to lowercase, which is problematic for case-sensitive databases
311+
final CaseInsensitiveHashMap<String> idMap = new CaseInsensitiveHashMap<>();
312+
rawIds.stream().map(String::valueOf).forEach(x -> idMap.put(x, x));
306313

307-
CaseInsensitiveHashMap<String> ciExistingAliases = new CaseInsensitiveHashMap<>();
308-
existingAliases.forEach((key, val) -> ciExistingAliases.put(String.valueOf(key), String.valueOf(val)));
314+
CaseInsensitiveHashMap<String> ciExistingAliases = new CaseInsensitiveHashMap<>();
315+
existingAliases.forEach((key, val) -> ciExistingAliases.put(String.valueOf(key), String.valueOf(val)));
309316

310-
SimpleFilter filter = new SimpleFilter(FieldKey.fromString("subjectname"), idMap.values(), CompareType.IN);
317+
SimpleFilter filter = new SimpleFilter(FieldKey.fromString("subjectname"), idMap.values(), CompareType.IN);
311318

312-
final Set<String> aliasesFound = new HashSet<>();
313-
TableInfo ti = getMappingTable();
314-
new TableSelector(ti, PageFlowUtil.set("subjectname", "externalAlias"), filter, null).forEachResults(rs -> {
315-
aliasesFound.add(rs.getString(FieldKey.fromString("subjectname")));
316-
if (ciExistingAliases.containsKey(rs.getString(FieldKey.fromString("subjectname")))) {
317-
if (!ciExistingAliases.get(rs.getString(FieldKey.fromString("subjectname"))).equalsIgnoreCase(rs.getString(FieldKey.fromString("externalAlias"))))
319+
final Set<String> aliasesFound = new HashSet<>();
320+
TableInfo ti = getMappingTable();
321+
new TableSelector(ti, PageFlowUtil.set("subjectname", "externalAlias"), filter, null).forEachResults(rs -> {
322+
aliasesFound.add(rs.getString(FieldKey.fromString("subjectname")));
323+
if (ciExistingAliases.containsKey(rs.getString(FieldKey.fromString("subjectname"))))
318324
{
319-
_log.error("Incoming MCC alias for: " + rs.getString(FieldKey.fromString("subjectname")) + "(" + ciExistingAliases.get(rs.getString(FieldKey.fromString("subjectname"))) + ") does not match existing: " + rs.getString(FieldKey.fromString("externalAlias")));
325+
if (!ciExistingAliases.get(rs.getString(FieldKey.fromString("subjectname"))).equalsIgnoreCase(rs.getString(FieldKey.fromString("externalAlias"))))
326+
{
327+
_log.error("Incoming MCC alias for: " + rs.getString(FieldKey.fromString("subjectname")) + "(" + ciExistingAliases.get(rs.getString(FieldKey.fromString("subjectname"))) + ") does not match existing: " + rs.getString(FieldKey.fromString("externalAlias")));
328+
}
320329
}
321-
}
322-
});
330+
});
323331

324-
aliasesFound.forEach(idMap::remove);
325-
if (idMap.isEmpty())
326-
{
327-
return 0;
328-
}
332+
aliasesFound.forEach(idMap::remove);
333+
if (idMap.isEmpty())
334+
{
335+
return 0;
336+
}
329337

330-
final List<Map<String, Object>> toAdd = new ArrayList<>();
331-
try
332-
{
333-
AtomicInteger aliasesReused = new AtomicInteger(0);
334-
idMap.forEach((key, id) -> {
335-
CaseInsensitiveHashMap<Object> row = new CaseInsensitiveHashMap<>();
336-
row.put("subjectname", id);
337-
if (ciExistingAliases.containsKey(id))
338+
final List<Map<String, Object>> toAdd = new ArrayList<>();
339+
try
340+
{
341+
AtomicInteger aliasesReused = new AtomicInteger(0);
342+
idMap.forEach((key, id) -> {
343+
CaseInsensitiveHashMap<Object> row = new CaseInsensitiveHashMap<>();
344+
row.put("subjectname", id);
345+
if (ciExistingAliases.containsKey(id))
346+
{
347+
_log.info("Will re-use existing MCC alias: " + ciExistingAliases.get(id) + ", for ID: " + id);
348+
aliasesReused.getAndIncrement();
349+
}
350+
351+
row.put("externalAlias", ciExistingAliases.get(id)); //NOTE: the trigger script will auto-assign a value if null, but we need to include this property on the input JSON
352+
353+
toAdd.add(row);
354+
});
355+
356+
if (!ciExistingAliases.isEmpty() && aliasesReused.get() != ciExistingAliases.size())
338357
{
339-
_log.info("Will re-use existing MCC alias: " + ciExistingAliases.get(id) + ", for ID: " + id);
340-
aliasesReused.getAndIncrement();
358+
_log.info("The existing aliases map, size: " + ciExistingAliases.size() + " does not equal the number of aliases actually used, which was: " + aliasesReused.get());
359+
_log.info(ciExistingAliases);
341360
}
342361

343-
row.put("externalAlias", ciExistingAliases.get(id)); //NOTE: the trigger script will auto-assign a value if null, but we need to include this property on the input JSON
362+
BatchValidationException bve = new BatchValidationException();
363+
ti.getUpdateService().insertRows(_user, _container, toAdd, bve, null, null);
364+
if (bve.hasErrors())
365+
{
366+
throw bve;
367+
}
344368

345-
toAdd.add(row);
346-
});
369+
transaction.commit();
347370

348-
if (!ciExistingAliases.isEmpty() && aliasesReused.get() != ciExistingAliases.size())
349-
{
350-
_log.info("The existing aliases map, size: " + ciExistingAliases.size() + " does not equal the number of aliases actually used, which was: " + aliasesReused.get());
351-
_log.info(ciExistingAliases);
371+
return toAdd.size();
352372
}
353-
354-
BatchValidationException bve = new BatchValidationException();
355-
ti.getUpdateService().insertRows(_user, _container, toAdd, bve, null, null);
356-
if (bve.hasErrors())
373+
catch (BatchValidationException | DuplicateKeyException | QueryUpdateServiceException | SQLException e)
357374
{
358-
throw bve;
375+
_log.error("Error auto-creating MCC aliases during insert for folder: " + _container.getPath(), e);
376+
toAdd.forEach(_log::error);
377+
return 0;
359378
}
360-
361-
return toAdd.size();
362-
}
363-
catch (BatchValidationException | DuplicateKeyException | QueryUpdateServiceException | SQLException e)
364-
{
365-
_log.error("Error auto-creating MCC aliases during insert for folder: " + _container.getPath(), e);
366-
toAdd.forEach(_log::error);
367-
return 0;
368379
}
369380
}
370381

@@ -391,9 +402,4 @@ public void updateDemographicsColony(String Id, String destination) throws Excep
391402
throw bve;
392403
}
393404
}
394-
395-
public void clearCachedTables()
396-
{
397-
_animalMapping = null;
398-
}
399405
}

0 commit comments

Comments
 (0)