Merge remote-tracking branch 'origin/develop' into fb_mcp_mvp

Author: labkey-adam

api/src/org/labkey/api/settings/OptionalFeatureFlag.java

@@ -12,24 +12,45 @@ public class OptionalFeatureFlag implements Comparable<OptionalFeatureFlag>, Sta
     private static final Logger LOG = LogHelper.getLogger(OptionalFeatureFlag.class, "Warnings about optional feature flag names");
 
     private final String _flag;
+    private final String _propertyName;
     private final String _title;
     private final String _description;
     private final boolean _requiresRestart;
     private final boolean _hidden;
     private final FeatureType _type;
 
     /**
-     * @param flag must be unique. Can be used as a startup property to enable/disable the task, but only if it follows
-     *             the Java identifier rules (e.g., alphanumeric plus _, start with a letter, no spaces).
+     * @param flag must be unique and conform to the Java identifier rules (e.g., alphanumeric plus _, start with a
+     *             letter, no spaces). That way it can be used as a startup property to enable/disable the task.
      */
     public OptionalFeatureFlag(String flag, String title, String description, boolean requiresRestart, boolean hidden, FeatureType type)
+    {
+        this(flag, title, description, requiresRestart, hidden, type, false);
+    }
+
+    OptionalFeatureFlag(String flag, String title, String description, boolean requiresRestart, boolean hidden, FeatureType type, boolean useDumbName /* true means allow a name that can't be used as a startup property name */)
     {
         _flag = flag;
         _title = title;
         _description = description;
         _requiresRestart = requiresRestart;
         _hidden = hidden;
         _type = type;
+        if (!StringUtilsLabKey.isValidJavaIdentifier(_flag))
+        {
+            if (useDumbName)
+            {
+                _propertyName = null;
+            }
+            else
+            {
+                throw new IllegalStateException(_flag + " is not a valid Java identifier. Correct it so it can be used as a startup property. Or set useDumbName to true.");
+            }
+        }
+        else
+        {
+            _propertyName = _flag;
+        }
     }
 
     public String getFlag()
@@ -83,14 +104,6 @@ public FeatureType getType()
     @Override
     public String getPropertyName()
     {
-        String name = getFlag();
-        if (!StringUtilsLabKey.isValidJavaIdentifier(name))
-        {
-            // This is a developer thing... no need to log a warning on production deployments
-            if (AppProps.getInstance().isDevMode())
-                LOG.warn("Feature flag name doesn't conform to the property name rules so it won't be available as a startup property: {}", name);
-            name = null;
-        }
-        return name;
+        return _propertyName;
     }
 }

api/src/org/labkey/api/settings/OptionalFeatureService.java

@@ -43,12 +43,21 @@ static void setInstance(OptionalFeatureService impl)
     }
 
     /**
-     * @param flag must be unique. Can be used as a startup property to enable/disable the task, but only if it follows
-     *             the Java identifier rules (e.g., alphanumeric plus _, start with a letter, no spaces).
+     * @param flag must be unique and conform to the Java identifier rules (e.g., alphanumeric plus _, start with a
+     *             letter, no spaces). That way it can be used as a startup property to enable/disable the task. If
+     *             you must use a flag that doesn't conform to these rules (why?) the call the other variant.
      */
     default void addExperimentalFeatureFlag(String flag, String title, String description, boolean requiresRestart)
     {
-        addFeatureFlag(new OptionalFeatureFlag(flag, title, description, requiresRestart, false, FeatureType.Experimental));
+        addExperimentalFeatureFlag(flag, title, description, requiresRestart, false);
+    }
+
+    /**
+     * This is left for backward compatibility. Use the variant above and provide flag that follows Java identifier rules.
+     */
+    default void addExperimentalFeatureFlag(String flag, String title, String description, boolean requiresRestart, boolean useDumbName)
+    {
+        addFeatureFlag(new OptionalFeatureFlag(flag, title, description, requiresRestart, false, FeatureType.Experimental, useDumbName));
     }
 
     void addFeatureFlag(OptionalFeatureFlag optionalFeatureFlag);

core/module.properties

@@ -1,6 +1,6 @@
 Name: Core
 ModuleClass: org.labkey.core.CoreModule
-SchemaVersion: 26.003
+SchemaVersion: 26.004
 Label: Administration and Essential Services
 Description: The Core module provides central services such as login, \
     security, administration, folder management, user management, \

core/resources/schemas/dbscripts/postgresql/core-25.008-25.009.sql

@@ -1,3 +1 @@
 ALTER TABLE core.Documents ADD ParentType VARCHAR(300);
-
-SELECT core.executeJavaUpgradeCode('populateAttachmentParentTypeColumn');

core/resources/schemas/dbscripts/postgresql/core-25.009-25.010.sql

@@ -4,5 +4,5 @@
 DELETE FROM core.Documents WHERE
     Container = (SELECT EntityId FROM core.Containers WHERE Parent IS NULL) AND
     Parent = (SELECT EntityId FROM core.Containers WHERE Parent IS NULL) AND
-    ParentType IS NULL AND
+    ParentType IS NULL AND -- ParentType is always NULL at this point, since populating the column is deferred. But the DocumentName condition below is sufficiently specific.
     (DocumentName LIKE 'auth_header_logo_%' OR DocumentName LIKE 'auth_login_page_logo_%');

core/resources/schemas/dbscripts/postgresql/core-26.000-26.001.sql

@@ -0,0 +1,3 @@
+-- Detection of the data class Compound.Structure2D attachment column was corrected in a recent PR: https://github.com/LabKey/platform/pull/7513
+-- This re-populates the ParentType column to pick up those changes.
+SELECT core.executeJavaUpgradeCode('populateAttachmentParentTypeColumn');

core/resources/schemas/dbscripts/postgresql/core-26.003-26.004.sql

@@ -1,3 +1 @@
 ALTER TABLE core.Documents ADD ParentType NVARCHAR(300);
-
-EXEC core.executeJavaUpgradeCode 'populateAttachmentParentTypeColumn';

core/resources/schemas/dbscripts/sqlserver/core-25.008-25.009.sql

@@ -4,5 +4,5 @@
 DELETE FROM core.Documents WHERE
     Container = (SELECT EntityId FROM core.Containers WHERE Parent IS NULL) AND
     Parent = (SELECT EntityId FROM core.Containers WHERE Parent IS NULL) AND
-    ParentType IS NULL AND
+    ParentType IS NULL AND -- ParentType is always NULL at this point, since populating the column is deferred. But the DocumentName condition below is sufficiently specific.
     (DocumentName LIKE 'auth_header_logo_%' OR DocumentName LIKE 'auth_login_page_logo_%');