Merge 25.8 to develop

XingY · XingY · commit 4173288dc73b · 2025-08-06T11:41:47.000-07:00
# Conflicts:
#	api/src/org/labkey/api/reader/DataLoader.java
diff --git a/api/src/org/labkey/api/assay/AssayService.java b/api/src/org/labkey/api/assay/AssayService.java
@@ -87,7 +87,8 @@ static void setInstance(AssayService impl)
     AssaySchema createSchema(User user, Container container, @Nullable Container targetStudy);
 
     /**
-     * @return all the assay protocols that are in scope in the given container
+     * @return all the assay protocols that are in scope in the given container, which may include those
+     * defined in other containers
      */
     @NotNull List<ExpProtocol> getAssayProtocols(Container container);
 
diff --git 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
@@ -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/exp/api/ExpObject.java b/api/src/org/labkey/api/exp/api/ExpObject.java
@@ -37,7 +37,7 @@ public interface ExpObject extends Identifiable, Comparable<ExpObject>
     String DEFAULT_CPAS_TYPE = "Object";
 
     /** Prevent edits to this object. Subsequent calls to setters will throw an IllegalStateException */
-    void lock();
+    ExpObject lock();
 
     int getRowId();
     /** Get the exp.object objectId */
diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java
@@ -1237,16 +1237,25 @@ private static void addDefaultUserLoginDelay(HttpServletRequest request, String
 
     // Throws UnauthorizedException if credentials are incorrect, password is expired, password is not complex enough,
     // user doesn't exist, user is inactive, or secondary auth is enabled and an API key hasn't been used.
-    public static @Nullable User authenticate(HttpServletRequest request, String id, String password) throws InvalidEmailException
+    public static @NotNull User authenticate(HttpServletRequest request, String id, String password) throws InvalidEmailException
     {
         PrimaryAuthenticationResult primaryResult = authenticate(request, id, password, null, true);
+        String message;
 
-        // If primary authentication is successful then look for secondary authentication. handleAuthentication() will
+        // If primary authentication is successful, then look for secondary authentication. handleAuthentication() will
         // always return a failure result (i.e., null user) if secondary authentication is enabled. #22944
         if (primaryResult.getStatus() == AuthenticationStatus.Success)
         {
             AuthenticationManager.setPrimaryAuthenticationResult(request, primaryResult);
-            return handleAuthentication(request, ContainerManager.getRoot()).getUser();
+            User user = handleAuthentication(request, ContainerManager.getRoot()).getUser();
+            if (user != null)
+                return user;
+
+            message = "Authentication failed because secondary authentication is enabled on this server. Consider using an API key instead.";
+        }
+        else
+        {
+            message = Objects.requireNonNullElse(primaryResult.getMessage(), primaryResult.getStatusErrorMessage(DisplayLocation.API));
         }
 
         // Basic auth has failed so send error response in a format that APIs can consume (JSON unless request specifies
@@ -1255,9 +1264,8 @@ private static void addDefaultUserLoginDelay(HttpServletRequest request, String
         // SpringActionController.handleRequest() will need to call this again since actions can specify the default
         // format.
         SpringActionController.setResponseFormat(request, Format.JSON);
-        String message = primaryResult.getMessage();
 
-        throw new UnauthorizedException(message != null ? message : primaryResult.getStatusErrorMessage(DisplayLocation.API));
+        throw new UnauthorizedException(message);
     }
 
     public static URLHelper logout(@NotNull User user, @NotNull HttpServletRequest request, URLHelper returnUrl)
diff --git a/api/src/org/labkey/api/security/SecurityPolicyManager.java b/api/src/org/labkey/api/security/SecurityPolicyManager.java
@@ -85,7 +85,7 @@ public class SecurityPolicyManager
                 Selector selector = new TableSelector(core.getTableInfoRoleAssignments(), filter, new Sort("UserId"));
                 Collection<RoleAssignment> assignments = selector.getCollection(RoleAssignment.class);
 
-                return new SecurityPolicy(policyBean.getResourceId(), policyBean.getResourceClass(), policyBean.getContainer().getId(), assignments, policyBean.getModified());
+                return new SecurityPolicy(policyBean.getResourceId(), policyBean.getResourceClass(), policyBean.getContainer().getId(), assignments.isEmpty() ? Collections.emptyList() : assignments, policyBean.getModified());
             }
 
             return null;
diff --git a/api/src/org/labkey/api/util/JsonUtil.java b/api/src/org/labkey/api/util/JsonUtil.java
@@ -24,13 +24,15 @@
 import com.fasterxml.jackson.core.json.JsonWriteFeature;
 import com.fasterxml.jackson.databind.*;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.mutable.MutableBoolean;
 import org.apache.logging.log4j.Logger;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.junit.Assert;
 import org.junit.Test;
 import org.labkey.api.action.ApiUsageException;
+import org.labkey.api.collections.CaseInsensitiveHashMap;
 import org.labkey.api.data.Container;
 import org.labkey.api.security.User;
 import org.labkey.api.settings.AppProps;
@@ -202,15 +204,20 @@ public static List<JSONObject> toJSONObjectList(JSONArray array)
         return result;
     }
 
-    // The new JSONObject.toMap() translates all JSONObjects into Maps and JSONArrays into Lists. In many cases, this is
-    // fine, but some existing code paths want to maintain the contained JSONObjects and JSONArrays. This method does
-    // that, acting more like the old JSONObject.toMap().
-    public static void fillMapShallow(JSONObject json, Map<String, Object> map)
+    /**
+     * The new JSONObject.toMap() translates all JSONObjects into Maps and JSONArrays into Lists. In many cases, this is
+     * fine, but some existing code paths want to maintain the contained JSONObjects and JSONArrays. This method does
+     * that, acting more like the old JSONObject.toMap().
+     * @return whether there were duplicate values in the map (useful when filling a case-insensitive map)
+      */
+    public static boolean fillMapShallow(JSONObject json, Map<String, Object> map)
     {
+        MutableBoolean hadDuplicates = new MutableBoolean(false);
         json.keySet().forEach(key -> {
             Object value = json.get(key);
-            map.put(key, JSONObject.NULL == value ? null : value);
+            hadDuplicates.setValue(map.put(key, JSONObject.NULL == value ? null : value) != null || hadDuplicates.booleanValue());
         });
+        return hadDuplicates.booleanValue();
     }
 
     // New JSONObject discards all properties with null values. This returns a JSONObject containing all Map values,
@@ -461,6 +468,20 @@ public void testJsonString()
             assertEquals(u.getUserId(), roundTripJson.get("user"));
         }
 
+        @Test
+        public void testConflictingCase()
+        {
+            JSONObject json = new JSONObject();
+            json.put("foo", "bar");
+            json.put("x", "y");
+            assertFalse(fillMapShallow(json, new HashMap<>()));
+            assertFalse(fillMapShallow(json, new CaseInsensitiveHashMap<>()));
+
+            json.put("FOO", "BAR");
+            assertFalse(fillMapShallow(json, new HashMap<>()));
+            assertTrue(fillMapShallow(json, new CaseInsensitiveHashMap<>()));
+        }
+
         @Test
         public void testSanitize()
         {
diff --git a/assay/src/org/labkey/assay/AssayManager.java b/assay/src/org/labkey/assay/AssayManager.java
@@ -138,6 +138,7 @@ public Set<String> getPermittedContainerIds(User user, Map<String, Container> co
         }
     };
 
+    /** Cache the protocols defined in a given container, which we can quickly compose to get the protocols in scope */
     private static final Cache<Container, List<ExpProtocol>> PROTOCOL_CACHE = CacheManager.getCache(CacheManager.UNLIMITED, TimeUnit.HOURS.toMillis(1), "Assay protocols");
 
     private static final List<AssayListener> _listeners = new CopyOnWriteArrayList<>();
@@ -287,7 +288,7 @@ private class ModuleAssayLsidHandlerFinder implements LsidHandlerFinder
 
         @Nullable
         @Override
-        public LsidHandler findHandler(String authority, String namespacePrefix)
+        public LsidHandler<?> findHandler(String authority, String namespacePrefix)
         {
             if (AppProps.getInstance().getDefaultLsidAuthority().equals(authority))
             {
@@ -360,29 +361,32 @@ public AssaySchema createSchema(User user, Container container, @Nullable Contai
     @Override
     public @NotNull List<ExpProtocol> getAssayProtocols(Container container)
     {
-        return PROTOCOL_CACHE.get(container, null, (c, argument) ->
-        {
-            // Build up a set of containers so that we can query them all at once
-            Set<Container> containers = c.getContainersFor(ContainerType.DataType.protocol);
-
-            List<? extends ExpProtocol> protocols = ExperimentService.get().getExpProtocols(containers.toArray(new Container[0]));
-            List<ExpProtocol> result = new ArrayList<>();
+        List<ExpProtocol> allProtocols = new ArrayList<>();
 
-            // Filter to just the ones that have an AssayProvider associated with them
-            for (ExpProtocol protocol : protocols)
+        for (Container containerInScope : container.getContainersFor(ContainerType.DataType.protocol))
+        {
+            List<ExpProtocol> ids = PROTOCOL_CACHE.get(containerInScope, null, (c, argument) ->
             {
-                AssayProvider p = getProvider(protocol);
-                if (p != null)
+                List<ExpProtocol> result = new ArrayList<>();
+
+                // Filter to just the ones that have an AssayProvider associated with them
+                for (ExpProtocol protocol : ExperimentService.get().getExpProtocols(c))
                 {
-                    // We don't want anyone editing our cached object
-                    protocol.lock();
-                    result.add(protocol);
+                    AssayProvider p = getProvider(protocol);
+                    if (p != null)
+                    {
+                        // We don't want anyone editing our cached object
+                        protocol.lock();
+                        result.add(protocol);
+                    }
                 }
-            }
-            // Sort them, just to be nice
-            Collections.sort(result);
-            return Collections.unmodifiableList(result);
-        });
+                return result.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(result);
+            });
+            allProtocols.addAll(ids);
+        }
+
+        Collections.sort(allProtocols);
+        return Collections.unmodifiableList(allProtocols);
     }
 
     @Override
diff --git a/experiment/src/org/labkey/experiment/api/ExpObjectImpl.java b/experiment/src/org/labkey/experiment/api/ExpObjectImpl.java
@@ -43,9 +43,10 @@ abstract public class ExpObjectImpl implements ExpObject, Serializable
     protected ExpObjectImpl() {}
 
     @Override
-    public void lock()
+    public ExpObjectImpl lock()
     {
         _locked = true;
+        return this;
     }
 
     protected void ensureUnlocked()
diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java
@@ -322,6 +322,7 @@ public class ExperimentServiceImpl implements ExperimentService, ObjectReference
 
     private final Cache<String, ExpProtocolImpl> PROTOCOL_LSID_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, CacheManager.HOUR, "Protocol by LSID",
         (key, argument) -> getExpProtocol(new SimpleFilter(FieldKey.fromParts("LSID"), key)));
+
     private final Cache<String, ExperimentRun> EXPERIMENT_RUN_CACHE = DatabaseCache.get(getExpSchema().getScope(), getTinfoExperimentRun().getCacheSize(), "Experiment Run by LSID", new ExperimentRunCacheLoader());
 
     private final Cache<String, SortedSet<DataClass>> dataClassCache = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "Data classes", (containerId, argument) ->
diff --git a/list/src/org/labkey/list/model/ListManager.java b/list/src/org/labkey/list/model/ListManager.java
@@ -108,7 +108,7 @@ public List<ListDef> load(@NotNull String entityId, @Nullable Object argument)
         {
             SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Container"), entityId);
             ArrayList<ListDef> ownLists = new TableSelector(getListMetadataTable(), filter, null).getArrayList(ListDef.class);
-            return Collections.unmodifiableList(ownLists);
+            return ownLists.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(ownLists);
         }
     }
 
diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java
@@ -4578,8 +4578,9 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole
             // NOTE RowMapFactory is faster, but for update it's important to preserve missing v explicit NULL values
             // Do we need to support some sort of UNDEFINED and NULL instance of MvFieldWrapper?
             RowMapFactory<Object> f = null;
-            if (commandType == CommandType.insert || commandType == CommandType.insertWithKeys)
+            if (commandType == CommandType.insert || commandType == CommandType.insertWithKeys || commandType == CommandType.delete)
                 f = new RowMapFactory<>();
+            CaseInsensitiveHashMap<Object> referenceCasing = new CaseInsensitiveHashMap<>();
 
             for (int idx = 0; idx < rows.length(); ++idx)
             {
@@ -4594,10 +4595,14 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole
                 }
                 if (null != jsonObj)
                 {
-                    // TODO: validate duplicate keys with different cases don't exist in data ('Name' vs 'name') (Issue 52616)
-                    Map<String, Object> rowMap = null == f ? new CaseInsensitiveHashMap<>() : f.getRowMap();
+                    Map<String, Object> rowMap = null == f ? new CaseInsensitiveHashMap<>(new HashMap<>(), referenceCasing) : f.getRowMap();
                     // Use shallow copy since jsonObj.toMap() will translate contained JSONObjects into Maps, which we don't want
-                    JsonUtil.fillMapShallow(jsonObj, rowMap);
+                    boolean conflictingCasing = JsonUtil.fillMapShallow(jsonObj, rowMap);
+                    if (conflictingCasing)
+                    {
+                        // Issue 52616
+                        LOG.error("Row contained conflicting casing for key names in the incoming row: {}", jsonObj);
+                    }
                     if (allowRowAttachments())
                         addRowAttachments(table, rowMap, idx, commandIndex);
 
diff --git a/query/src/org/labkey/query/persist/ExternalSchemaDefCache.java b/query/src/org/labkey/query/persist/ExternalSchemaDefCache.java
diff --git a/query/src/org/labkey/query/reports/DatabaseReportCache.java b/query/src/org/labkey/query/reports/DatabaseReportCache.java
diff --git a/study/test/src/org/labkey/test/tests/study/StudyPHIExportTest.java b/study/test/src/org/labkey/test/tests/study/StudyPHIExportTest.java

Original file line number	Diff line number	Diff line change
`@@ -96,11 +96,11 @@ public boolean next()`
`96`	`96`
`97`	`97`	`if (null == attachmentValue)`
`98`	`98`	`continue;`
`99`		`- else if (attachmentValue instanceof String)`
	`99`	`+ else if (attachmentValue instanceof String str)`
`100`	`100`	`{`
`101`	`101`	`if (null == attachmentDir)`
`102`	`102`	`{`
`103`		`- errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Can't upload to field " + p.domainProperty.getName() + " with type " + p.domainProperty.getType().getLabel() + "."));`
	`103`	`+ errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Can't upload '" + str + "' to field " + p.domainProperty.getName() + " with type " + p.domainProperty.getType().getLabel() + "."));`
`104`	`104`	`return false;`
`105`	`105`	`}`
`106`	`106`	`filename = (String) attachmentValue;`
`@@ -124,7 +124,13 @@ else if (attachmentValue instanceof File)`
`124`	`124`	`}`
`125`	`125`	`else`
`126`	`126`	`{`
`127`		`- errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachament file."));`
	`127`	`+ errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file."));`
	`128`	`+ return false;`
	`129`	`+ }`
	`130`	`+`
	`131`	`+ if (entityIdIndex == 0)`
	`132`	`+ {`
	`133`	`+ errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file."));`
`128`	`134`	`return false;`
`129`	`135`	`}`
`130`	`136`
`@@ -139,7 +145,7 @@ else if (attachmentValue instanceof File)`
`139`	`145`	`if (null != attachmentFiles && !attachmentFiles.isEmpty())`
`140`	`146`	`{`
`141`	`147`	`String entityId = String.valueOf(get(entityIdIndex));`
`142`		`- AttachmentService.get().addAttachments(getAttachmentParent(entityId, container) , attachmentFiles, user);`
	`148`	`+ AttachmentService.get().addAttachments(getAttachmentParent(entityId, container), attachmentFiles, user);`
`143`	`149`	`}`
`144`	`150`	`return ret;`
`145`	`151`	`}`
`@@ -217,7 +223,7 @@ public static DataIteratorBuilder getAttachmentDataIteratorBuilder(TableInfo ti,`
`217`	`223`	`}`
`218`	`224`	`}`
`219`	`225`
`220`		`- if (!attachmentColumns.isEmpty() && 0 != entityIdIndex)`
	`226`	`+ if (!attachmentColumns.isEmpty())`
`221`	`227`	`return new AttachmentDataIterator(it, context.getErrors(), user, attachmentDir, entityIdIndex, attachmentColumns, context.getInsertOption(), container, parentFactory );`
`222`	`228`
`223`	`229`	`return it;`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ public class SecurityPolicyManager`
`85`	`85`	`Selector selector = new TableSelector(core.getTableInfoRoleAssignments(), filter, new Sort("UserId"));`
`86`	`86`	`Collection<RoleAssignment> assignments = selector.getCollection(RoleAssignment.class);`
`87`	`87`
`88`		`- return new SecurityPolicy(policyBean.getResourceId(), policyBean.getResourceClass(), policyBean.getContainer().getId(), assignments, policyBean.getModified());`
	`88`	`+ return new SecurityPolicy(policyBean.getResourceId(), policyBean.getResourceClass(), policyBean.getContainer().getId(), assignments.isEmpty() ? Collections.emptyList() : assignments, policyBean.getModified());`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`return null;`
Original file line number	Diff line number	Diff line change
`@@ -43,9 +43,10 @@ abstract public class ExpObjectImpl implements ExpObject, Serializable`
`43`	`43`	`protected ExpObjectImpl() {}`
`44`	`44`
`45`	`45`	`@Override`
`46`		`- public void lock()`
	`46`	`+ public ExpObjectImpl lock()`
`47`	`47`	`{`
`48`	`48`	`_locked = true;`
	`49`	`+ return this;`
`49`	`50`	`}`
`50`	`51`
`51`	`52`	`protected void ensureUnlocked()`