diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index dc3ad61edb7..bb27bd13b15 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -40,6 +40,7 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.ContainerService; import org.labkey.api.data.DatabaseIdentifier; +import org.labkey.api.data.DbScope; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.PHI; import org.labkey.api.data.PropertyStorageSpec; @@ -107,6 +108,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.ListIterator; @@ -117,6 +119,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.labkey.api.data.ColumnRenderPropertiesImpl.TEXT_CHOICE_CONCEPT_URI; import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior; @@ -1050,6 +1053,8 @@ else if (PropertyType.MULTI_CHOICE.getTypeUri().equals(old.getRangeURI())) { for (Map valueUpdate : entry.getValue()) updateTextChoiceValueRows(d, user, entry.getKey(), valueUpdate, validationException); + if (!validationException.getErrors().isEmpty()) + return validationException; } // update indices - add missing and drop those that aren't included in domain info @@ -1409,6 +1414,8 @@ else if (prop == null) return Pair.of(valueUpdates, deletedValues); } + private static final int TEXT_CHOICE_UPDATE_BATCH_SIZE = 1000; + private static void updateTextChoiceValueRows(Domain domain, User user, DomainProperty prop, Map valueUpdates, ValidationException errors) { if (domain != null && domain.getDomainKind() != null) @@ -1421,57 +1428,49 @@ private static void updateTextChoiceValueRows(Domain domain, User user, DomainPr TableInfo domainTable = domain.getDomainKind().getTableInfo(user, domain.getContainer(), domain, ContainerFilter.getUnsafeEverythingFilter()); if (domainTable != null && domainTable.getUpdateService() != null) { - // we need to make all the row updates for this domain property at one time to prevent the - // double mapping if one choice value was changed from a -> b and another from b -> c - // (not sure why someone would do that though) - List> rows = new ArrayList<>(); - - for (Map.Entry entry : valueUpdates.entrySet()) + List columns = new ArrayList<>(domainTable.getPkColumns()); + ColumnInfo propCol = domainTable.getColumn(propName); + if (propCol == null) { - // query for the row PKs of domain rows that have the original text choice value - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(propName), entry.getKey()); - // filter out aliquots for sample type domain - if (domain.getDomainKind() instanceof SampleTypeDomainKind && isParentOnlyField) - filter.addCondition(FieldKey.fromParts("IsAliquot"), false); - List columns = new ArrayList<>(domainTable.getPkColumns()); - if (domainTable.getContainerFieldKey() != null) - columns.add(domainTable.getColumn(domainTable.getContainerFieldKey())); - var resultsetRows = new TableSelector(domainTable, columns, filter, null).getMapCollection(); - - // generate a column name map for updateRow(), add the updated property value into the row map as well - for (Map rsRow : resultsetRows) + errors.addError(new PropertyValidationError("Property column not found", propName)); + return; + } + columns.add(propCol); + ColumnInfo containerCol = domainTable.getContainerFieldKey() != null ? domainTable.getColumn(domainTable.getContainerFieldKey()) : null; + if (containerCol != null) + columns.add(containerCol); + + SimpleFilter filter = new SimpleFilter(new SimpleFilter.InClause(propCol.getFieldKey(), valueUpdates.keySet())); + // filter out aliquots for sample type domain + if (domain.getDomainKind() instanceof SampleTypeDomainKind && isParentOnlyField) + filter.addCondition(FieldKey.fromParts("IsAliquot"), false); + + Map containerTables = new HashMap<>(); + BatchValidationException batchErrors = new BatchValidationException(); + // The transaction makes the PostgreSQL driver use a cursor; the cursor's snapshot reads each row once with its original value, so a -> b, b -> c (or a swap) can't double map + try (DbScope.Transaction transaction = domainTable.getSchema().getScope().ensureTransaction(); + Stream> stream = new TableSelector(domainTable, columns, filter, null).uncachedMapStream()) + { + List> batch = new ArrayList<>(TEXT_CHOICE_UPDATE_BATCH_SIZE); + Iterator> iter = stream.iterator(); + while (iter.hasNext()) { + Map rsRow = iter.next(); var valueRow = new CaseInsensitiveHashMap<>(); for (ColumnInfo col : columns) valueRow.put(col.getName(), col.getValue(rsRow)); - valueRow.put(propName, entry.getValue()); - rows.add(valueRow); - } - } + valueRow.put(propCol.getName(), valueUpdates.get((String) propCol.getValue(rsRow))); + batch.add(valueRow); - try - { - BatchValidationException batchErrors = new BatchValidationException(); - // use update rows against each distinct row container to map the text choice value to the updated value, - // using each row container so that the audit events end up in the right container - if (domainTable.getContainerFieldKey() != null) - { - String containerFieldName = domainTable.getContainerFieldKey().getName(); - Set rowContainers = rows.stream().map((row) -> (String) row.get(containerFieldName)).collect(Collectors.toSet()); - for (String rowContainer : rowContainers) + if (batch.size() == TEXT_CHOICE_UPDATE_BATCH_SIZE || !iter.hasNext()) { - // GitHub Issue 924: Updating Single Text choice values errors when there are child folders - var dataContainer = ContainerManager.getForId(rowContainer); - var domainTable_ = domain.getDomainKind().getTableInfo(user, dataContainer, domain, ContainerFilter.getUnsafeEverythingFilter()); - List> containerRows = rows.stream().filter((row) -> row.get(containerFieldName).equals(rowContainer)).collect(Collectors.toList()); - domainTable_.getUpdateService().updateRows(user, dataContainer, containerRows, containerRows, batchErrors, Map.of(AuditBehavior, AuditBehaviorType.DETAILED), null); + updateTextChoiceValueBatch(domain, user, domainTable, containerCol, containerTables, batch, batchErrors); + if (batchErrors.hasErrors()) + throw batchErrors; + batch = new ArrayList<>(TEXT_CHOICE_UPDATE_BATCH_SIZE); } } - else - domainTable.getUpdateService().updateRows(user, domain.getContainer(), rows, rows, batchErrors, Map.of(AuditBehavior, AuditBehaviorType.DETAILED), null); - - if (batchErrors.hasErrors()) - throw batchErrors; + transaction.commit(); } catch (Exception e) { @@ -1481,6 +1480,27 @@ private static void updateTextChoiceValueRows(Domain domain, User user, DomainPr } } + private static void updateTextChoiceValueBatch(Domain domain, User user, TableInfo domainTable, @Nullable ColumnInfo containerCol, Map containerTables, + List> batch, BatchValidationException batchErrors) throws Exception + { + if (containerCol == null) + { + domainTable.getUpdateService().updateRows(user, domain.getContainer(), batch, batch, batchErrors, Map.of(AuditBehavior, AuditBehaviorType.DETAILED), null); + return; + } + + Map>> rowsByContainer = batch.stream().collect(Collectors.groupingBy(row -> (String) row.get(containerCol.getName()))); + for (Map.Entry>> entry : rowsByContainer.entrySet()) + { + // GitHub Issue 924: Updating Single Text choice values errors when there are child folders + Container dataContainer = ContainerManager.getForId(entry.getKey()); + TableInfo table = containerTables.computeIfAbsent(entry.getKey(), + id -> domain.getDomainKind().getTableInfo(user, dataContainer, domain, ContainerFilter.getUnsafeEverythingFilter())); + List> rows = entry.getValue(); + table.getUpdateService().updateRows(user, dataContainer, rows, rows, batchErrors, Map.of(AuditBehavior, AuditBehaviorType.DETAILED), null); + } + } + private static boolean _copyValidator(IPropertyValidator pv, GWTPropertyValidator gpv) { boolean hasChange = false;