diff --git a/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java b/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java index 926f6a516e8..c8caccc0d6c 100644 --- a/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java @@ -96,11 +96,11 @@ public boolean next() if (null == attachmentValue) continue; - else if (attachmentValue instanceof String) + else if (attachmentValue instanceof String str) { if (null == attachmentDir) { - errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Can't upload to field " + p.domainProperty.getName() + " with type " + p.domainProperty.getType().getLabel() + ".")); + errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Can't upload '" + str + "' to field " + p.domainProperty.getName() + " with type " + p.domainProperty.getType().getLabel() + ".")); return false; } filename = (String) attachmentValue; @@ -124,7 +124,13 @@ else if (attachmentValue instanceof File) } else { - errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachament file.")); + errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file.")); + return false; + } + + if (entityIdIndex == 0) + { + errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file.")); return false; } @@ -139,7 +145,7 @@ else if (attachmentValue instanceof File) if (null != attachmentFiles && !attachmentFiles.isEmpty()) { String entityId = String.valueOf(get(entityIdIndex)); - AttachmentService.get().addAttachments(getAttachmentParent(entityId, container) , attachmentFiles, user); + AttachmentService.get().addAttachments(getAttachmentParent(entityId, container), attachmentFiles, user); } return ret; } @@ -217,7 +223,7 @@ public static DataIteratorBuilder getAttachmentDataIteratorBuilder(TableInfo ti, } } - if (!attachmentColumns.isEmpty() && 0 != entityIdIndex) + if (!attachmentColumns.isEmpty()) return new AttachmentDataIterator(it, context.getErrors(), user, attachmentDir, entityIdIndex, attachmentColumns, context.getInsertOption(), container, parentFactory ); return it; diff --git a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java index d4f7d0e697a..dfbf6b0150d 100644 --- a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java +++ b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java @@ -277,8 +277,10 @@ public DataIterator getDataIterator(DataIteratorContext context) PropertyType pt = pd == null ? null : pd.getPropertyType(); boolean isAttachment = pt == PropertyType.ATTACHMENT || pt == PropertyType.FILE_LINK; - if (null == pair.target || isAttachment) + if (null == pair.target) convert.addColumn(pair.indexFrom); + else if (isAttachment) // Issue 53498: attachment is blank after update from file, if the field name contains underscore + convert.addColumn(pair.target, pair.indexFrom); else convert.addConvertColumn(pair.target, pair.indexFrom, pair.indexMv, pd, pt, pair.target.getRemapMissingBehavior(), context.isWithLookupRemapping()); } diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index ea020b9ec6a..d1cedc7ae7d 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -43,6 +43,7 @@ import org.labkey.api.dataiterator.ScrollableDataIterator; import org.labkey.api.exp.MvColumn; import org.labkey.api.exp.MvFieldWrapper; +import org.labkey.api.exp.PropertyType; import org.labkey.api.iterator.CloseableFilteredIterator; import org.labkey.api.iterator.CloseableIterator; import org.labkey.api.query.BatchValidationException; @@ -327,7 +328,7 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws for (int f = 0; f < nCols; f++) { List classesToTest = new ArrayList<>(Arrays.asList(CONVERT_CLASSES)); - Class knownColumnClass = null; + Class knownColumnClass; int classIndex = -1; //NOTE: this means we have a header row @@ -337,27 +338,21 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws { String name = lineFields[0][f]; name = StringUtilsLabKey.sanitizeSeparatorsAndTrim(name); - if (_columnInfoMap.containsKey(name)) - { - //preferentially use this class if it matches - knownColumnClass = _columnInfoMap.get(name).getJavaClass(); - classesToTest.add(0, knownColumnClass); - } - else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedColumns.get(name))) + + ColumnInfo knownColumn = getKnownColumn(name, renamedColumns); + + if (knownColumn != null) { - knownColumnClass = _columnInfoMap.get(renamedColumns.get(name)).getJavaClass(); + knownColumnClass = knownColumn.getJavaClass(); classesToTest.add(0, knownColumnClass); + + // Issue 49830: if we know the column class is File based on the columnInfoMap, use it instead of trying to infer the class based on the data + if (setFileColDescriptor(colDescs[f], knownColumn)) + continue; } } } - // Issue 49830: if we know the column class is File based on the columnInfoMap, use it instead of trying to infer the class based on the data - if (File.class.equals(knownColumnClass)) - { - colDescs[f].clazz = knownColumnClass; - continue; - } - for (int line = inferStartLine; line < numLines; line++) { if (f >= lineFields[line].length) @@ -458,7 +453,8 @@ else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedC Set columnNames = new HashSet<>(); for (ColumnDescriptor colDesc : colDescs) { - if (!columnNames.add(colDesc.name) && isThrowOnErrors()) + String name = colDesc.name; + if (!columnNames.add(name) && isThrowOnErrors()) { // TODO: This should be refactored to not throw this here, but rather, have the callers check themselves. It // is not in the interest of inferring columns that we validate duplicate columns. @@ -468,11 +464,45 @@ else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedC ExceptionUtil.decorateException(e, ExceptionUtil.ExceptionInfo.SkipMothershipLogging, "true", true); throw e; } + + // use File converter for known file fields even if inferTypes = false. If inferTypes, this is already done. + if (!getInferTypes()) + setFileColDescriptor(colDesc, getKnownColumn(name, renamedColumns)); } _columns = colDescs; } + private boolean setFileColDescriptor(ColumnDescriptor colDesc, @Nullable ColumnInfo knownColumn) + { + if (knownColumn == null) + return false; + + Class knownColumnClass = knownColumn.getJavaClass(); + + if (File.class.equals(knownColumnClass)) + { + if (PropertyType.ATTACHMENT.equals(knownColumn.getPropertyType())) + colDesc.clazz = String.class; + else + colDesc.clazz = knownColumnClass; + + return true; + } + + return false; + } + + private ColumnInfo getKnownColumn(String name, @NotNull Map renamedColumns) + { + ColumnInfo knownColumn = null; + if (_columnInfoMap.containsKey(name)) + knownColumn = _columnInfoMap.get(name); + else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedColumns.get(name))) + knownColumn = _columnInfoMap.get(renamedColumns.get(name)); + return knownColumn; + } + protected String getDefaultColumnName(int col) { return "column" + col; @@ -955,7 +985,16 @@ public DataIterator getDataIterator(final DataIteratorContext context) /** Actually create an instance of DataIterator to use, which might be subclass-specific */ protected DataIterator createDataIterator(DataIteratorContext context) throws IOException { - return new _DataIterator(context, getActiveColumns(), isScrollable()); + ColumnDescriptor[] columnDescriptors = getActiveColumns(); + if (context.isCrossFolderImport() || context.isCrossTypeImport()) + { + for (ColumnDescriptor columnDescriptor : columnDescriptors) + { + if (columnDescriptor.clazz.equals(File.class)) + columnDescriptor.clazz = String.class; // defer file path validation for cross type/folder import + } + } + return new _DataIterator(context, columnDescriptors, isScrollable()); } protected class _DataIterator implements ScrollableDataIterator, MapDataIterator