Refactor AdminConsole vs. OptionalFeatureService

.gitattributes

@@ -78,27 +78,17 @@ api/gwtsrc/org/labkey/api/gwt/client/model/GWTIndex.java -text
 api/gwtsrc/org/labkey/api/gwt/client/model/GWTPropertyDescriptor.java -text
 api/gwtsrc/org/labkey/api/gwt/client/model/GWTPropertyValidator.java -text
 api/gwtsrc/org/labkey/api/gwt/client/model/PropertyValidatorType.java -text
-api/gwtsrc/org/labkey/api/gwt/client/pipeline/GWTPipelineConfig.java -text
-api/gwtsrc/org/labkey/api/gwt/client/pipeline/GWTPipelineTask.java -text
-api/gwtsrc/org/labkey/api/gwt/client/pipeline/PipelineGWTService.java -text
-api/gwtsrc/org/labkey/api/gwt/client/pipeline/PipelineGWTServiceAsync.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/BoundTextBox.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/DirtyCallback.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/domain/CancellationException.java -text
-api/gwtsrc/org/labkey/api/gwt/client/ui/domain/ImportStatus.java -text
-api/gwtsrc/org/labkey/api/gwt/client/ui/FontButton.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/HelpPopup.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/ImageButton.java -text
-api/gwtsrc/org/labkey/api/gwt/client/ui/LinkButton.java -text
-api/gwtsrc/org/labkey/api/gwt/client/ui/StringListBox.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/TextBoxDialogBox.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/WebPartPanel.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/WidgetUpdatable.java -text
 api/gwtsrc/org/labkey/api/gwt/client/ui/WindowUtil.java -text
 api/gwtsrc/org/labkey/api/gwt/client/util/BooleanProperty.java -text
 api/gwtsrc/org/labkey/api/gwt/client/util/ColorGenerator.java -text
-api/gwtsrc/org/labkey/api/gwt/client/util/DateProperty.java -text
-api/gwtsrc/org/labkey/api/gwt/client/util/DoubleProperty.java -text
 api/gwtsrc/org/labkey/api/gwt/client/util/ErrorDialogAsyncCallback.java -text
 api/gwtsrc/org/labkey/api/gwt/client/util/IntegerProperty.java -text
 api/gwtsrc/org/labkey/api/gwt/client/util/IPropertyWrapper.java -text
@@ -197,7 +187,6 @@ api/src/org/labkey/api/admin/AdminConsoleService.java -text
 api/src/org/labkey/api/admin/AdminUrls.java -text
 api/src/org/labkey/api/admin/BaseFolderWriter.java -text
 api/src/org/labkey/api/admin/CoreUrls.java -text
-api/src/org/labkey/api/admin/DiagnosticsLink.java -text
 api/src/org/labkey/api/admin/FolderArchiveDataTypes.java -text
 api/src/org/labkey/api/admin/FolderExportContext.java -text
 api/src/org/labkey/api/admin/FolderExportPermission.java -text
@@ -466,7 +455,6 @@ api/src/org/labkey/api/data/dialect/StandardJdbcMetaDataLocator.java -text
 api/src/org/labkey/api/data/dialect/StandardTableResolver.java -text
 api/src/org/labkey/api/data/dialect/StatementWrapper.java -text
 api/src/org/labkey/api/data/dialect/TableResolver.java -text
-api/src/org/labkey/api/data/dialect/TestUpgradeCode.java -text
 api/src/org/labkey/api/data/DimensionColumnInfo.java -text
 api/src/org/labkey/api/data/DisplayColumnDecorator.java -text
 api/src/org/labkey/api/data/DisplayColumnFactory.java -text
@@ -1137,7 +1125,6 @@ api/src/org/labkey/api/security/ActionNames.java -text
 api/src/org/labkey/api/security/AdminConsoleAction.java -text
 api/src/org/labkey/api/security/ApiKeyManager.java -text
 api/src/org/labkey/api/security/AuthenticatedRequest.java -text
-api/src/org/labkey/api/security/AuthenticatedResponse.java -text
 api/src/org/labkey/api/security/AuthenticationLogoAttachmentParent.java -text
 api/src/org/labkey/api/security/AuthenticationLogoType.java -text
 api/src/org/labkey/api/security/AuthenticationManager.java -text
@@ -2352,7 +2339,6 @@ pipeline/src/org/labkey/pipeline/permission.jsp -text
 pipeline/src/org/labkey/pipeline/PipelineCommandTestCase.java -text
 pipeline/src/org/labkey/pipeline/PipelineController.java -text
 pipeline/src/org/labkey/pipeline/PipelineDirectoryImpl.java -text
-pipeline/src/org/labkey/pipeline/PipelineGWTServiceImpl.java -text
 pipeline/src/org/labkey/pipeline/PipelineModule.java -text
 pipeline/src/org/labkey/pipeline/PipelineWebdavProvider.java -text
 pipeline/src/org/labkey/pipeline/PipelineWebPart.java -text
@@ -2906,7 +2892,6 @@ study/test/src/org/labkey/test/tests/study/StudyDemoModeTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyExportTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyLotsOfParticipantsTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyManualTest.java -text
-study/test/src/org/labkey/test/tests/study/StudyMergeParticipantsTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyProtocolDesignerTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyPublishTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyReloadColumnInferenceTest.java -text

api/src/org/labkey/api/ApiModule.java

@@ -138,16 +138,14 @@
 import org.labkey.api.security.SecurityManager;
 import org.labkey.api.security.UserManager;
 import org.labkey.api.security.ValidEmail;
-import org.labkey.api.settings.AdminConsole;
-import org.labkey.api.settings.AdminConsole.OptionalFeatureFlag;
 import org.labkey.api.settings.AppProps;
 import org.labkey.api.settings.AppPropsTestCase;
 import org.labkey.api.settings.BaseServerProperties;
 import org.labkey.api.settings.LookAndFeelFolderPropertiesTest;
 import org.labkey.api.settings.LookAndFeelProperties;
+import org.labkey.api.settings.OptionalFeatureFlag;
 import org.labkey.api.settings.OptionalFeatureService;
 import org.labkey.api.settings.OptionalFeatureService.FeatureType;
-import org.labkey.api.settings.OptionalFeatureStartupListener;
 import org.labkey.api.settings.WriteableLookAndFeelProperties;
 import org.labkey.api.util.ChecksumUtil;
 import org.labkey.api.util.Compress;
@@ -232,11 +230,11 @@ protected void init()
 
         LabKeyManagement.register(new StandardMBean(new OperationsMXBeanImpl(), OperationsMXBean.class, true), "Operations");
 
-        AdminConsole.addOptionalFeatureFlag(new OptionalFeatureFlag(FileStream.STAGE_FILE_TRANSFERS,
+        OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(FileStream.STAGE_FILE_TRANSFERS,
             "Stage file uploads and downloads to temporary local file",
             "When using a non-local file system, using a specific API that requires a locally staged copy of the file as the source can sometimes be significantly faster than streaming the file directly to/from storage",
             false, false, FeatureType.Optional));
-        AdminConsole.addOptionalFeatureFlag(new OptionalFeatureFlag(MothershipReport.FEATURE_FLAG_EXTENDED_METRICS,
+        OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(MothershipReport.FEATURE_FLAG_EXTENDED_METRICS,
             "Send extended metrics information to LabKey",
             "Send additional usage information to https://www.labkey.org along with standard usage metrics. This " +
             "information includes a list of unique, active users registered on the server. Providing this information " +
@@ -263,9 +261,7 @@ protected void doStartup(ModuleContext moduleContext)
         ContentSecurityPolicyFilter.registerMetricsProvider();
         ApiKeyManager.get().handleStartupProperties();
         MailHelper.init();
-        // Handle optional feature and system maintenance startup properties as late as possible; we want all optional
-        // features and system maintenance tasks to be registered first
-        ContextListener.addStartupListener(new OptionalFeatureStartupListener());
+        // Handle system maintenance startup properties as late as possible; we want all system maintenance tasks to be registered first
         ContextListener.addStartupListener(new SystemMaintenanceStartupListener());
         ContextListener.addStartupListener(new StartupPropertyStartupListener());
     }

api/src/org/labkey/api/module/CodeOnlyModule.java

@@ -25,13 +25,9 @@
 import java.util.Set;
 
 /**
- * Bit of a misnomer, but I couldn't think of a better name. These modules provide code and resources, but don't manage
- * any schemas, don't run SQL scripts, and don't need to do anything at upgrade time. This simplifies the implementation
- * of such modules.
- *
- * Perhaps this should implement Module instead of extending DefaultModule.
- *
- * Created by adam on 6/29/2016.
+ * This is a bit of a misnomer, but I couldn't think of a better name. These modules provide code and resources, but
+ * don't manage any schemas, don't run SQL scripts, and don't need to do anything at upgrade time. This simplifies the
+ * implementation of such modules. Perhaps this should implement Module instead of extending DefaultModule.
  */
 public abstract class CodeOnlyModule extends DefaultModule
 {

api/src/org/labkey/api/query/AbstractQueryUpdateService.java

@@ -125,7 +125,8 @@
 
 public abstract class AbstractQueryUpdateService implements QueryUpdateService
 {
-    private TableInfo _queryTable;
+    private final TableInfo _queryTable;
+
     private boolean _bulkLoad = false;
     private CaseInsensitiveHashMap<ColumnInfo> _columnImportMap = null;
     private VirtualFile _att = null;
@@ -159,7 +160,7 @@ protected TableInfo getQueryTable()
     }
 
     @Override
-    public boolean hasPermission(@NotNull UserPrincipal user, Class<? extends Permission> acl)
+    public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class<? extends Permission> acl)
     {
         return getQueryTable().hasPermission(user, acl);
     }
@@ -261,8 +262,8 @@ protected final DataIteratorContext getDataIteratorContext(BatchValidationExcept
     }
 
     /**
-     * if QUS want to use something other than PKs to select existing rows for merge it can override this method
-     * Used only for generating  ExistingRecordDataIterator at the moment
+     * If QUS wants to use something other than PKs to select existing rows for merge, it can override this method.
+     * Used only for generating ExistingRecordDataIterator at the moment.
      */
     protected Set<String> getSelectKeys(DataIteratorContext context)
     {
@@ -295,7 +296,7 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI
     /**
      * Implementation to use insertRows() while we migrate to using DIB for all code paths
      * <p>
-     * DataIterator should/must use same error collection as passed in
+     * DataIterator should/must use the same error collection as passed in
      */
     @Deprecated
     protected int _importRowsUsingInsertRows(User user, Container container, DataIterator rows, BatchValidationException errors, Map<String, Object> extraScriptContext)

api/src/org/labkey/api/query/QueryUpdateService.java

@@ -319,6 +319,6 @@ int truncateRows(User user, Container container, @Nullable Map<Enum, Object> con
      */
     boolean isBulkLoad();
 
-    /** Setup the data iterator for any special behavior needed for the target table */
+    /** Set up the data iterator for any special behavior needed for the target table */
     default void configureDataIteratorContext(DataIteratorContext context) {}
 }

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

@@ -15,27 +15,19 @@
  */
 package org.labkey.api.settings;
 
-import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.security.User;
 import org.labkey.api.security.permissions.Permission;
-import org.labkey.api.settings.OptionalFeatureService.FeatureType;
-import org.labkey.api.util.StringUtilsLabKey;
-import org.labkey.api.util.logging.LogHelper;
 import org.labkey.api.view.ActionURL;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.TreeSet;
-import java.util.concurrent.ConcurrentSkipListSet;
 
 /**
  * Manages registration for links to be shown in the Admin Console, as well as experimental features that can
@@ -63,9 +55,7 @@ public String getCaption()
         }
     }
 
-    private static final Logger LOG = LogHelper.getLogger(AdminConsole.class, "Warnings about optional feature flag names");
     private static final Map<SettingsLinkType, Collection<AdminLink>> _links = new HashMap<>();
-    private static final Set<OptionalFeatureFlag> _optionalFlags = new ConcurrentSkipListSet<>();
 
     static
     {
@@ -142,116 +132,4 @@ public int compareTo(@NotNull AdminLink o)
             return getText().compareToIgnoreCase(o.getText());
         }
     }
-
-    /**
-     * @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).
-     */
-    public static void addExperimentalFeatureFlag(String flag, String title, String description, boolean requiresRestart)
-    {
-        addOptionalFeatureFlag(new OptionalFeatureFlag(flag, title, description, requiresRestart, false, FeatureType.Experimental));
-    }
-
-    public static void addOptionalFeatureFlag(OptionalFeatureFlag optionalFeatureFlag)
-    {
-        _optionalFlags.add(optionalFeatureFlag);
-    }
-
-    // Return all optional features, regardless of type
-    public static Collection<OptionalFeatureFlag> getOptionalFeatureFlags()
-    {
-        return Collections.unmodifiableSet(_optionalFlags);
-    }
-
-    // Return all optional features having the specified type
-    public static Collection<OptionalFeatureFlag> getOptionalFeatureFlags(FeatureType type)
-    {
-        return _optionalFlags.stream()
-            .filter(flag -> flag.getType() == type)
-            .toList();
-    }
-
-    public static class OptionalFeatureFlag implements Comparable<OptionalFeatureFlag>, StartupProperty
-    {
-        private final String _flag;
-        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).
-         */
-        public OptionalFeatureFlag(String flag, String title, String description, boolean requiresRestart, boolean hidden, FeatureType type)
-        {
-            _flag = flag;
-            _title = title;
-            _description = description;
-            _requiresRestart = requiresRestart;
-            _hidden = hidden;
-            _type = type;
-        }
-
-        public String getFlag()
-        {
-            return _flag;
-        }
-
-        public String getTitle()
-        {
-            return _title;
-        }
-
-        @Override
-        public String getDescription()
-        {
-            return _description;
-        }
-
-        public boolean isRequiresRestart()
-        {
-            return _requiresRestart;
-        }
-
-        @Override
-        public int compareTo(@NotNull AdminConsole.OptionalFeatureFlag o)
-        {
-            return getTitle().compareToIgnoreCase(o.getTitle());
-        }
-
-        public boolean isEnabled()
-        {
-            return AppProps.getInstance().isOptionalFeatureEnabled(getFlag());
-        }
-
-        public boolean isHidden()
-        {
-            return _hidden;
-        }
-
-        public FeatureType getType()
-        {
-            return _type;
-        }
-
-        // StartupProperty implementation
-
-        /**
-         *  Returns {@code null} if {@code getFlag()} does not conform to the property name rules.
-         */
-        @Nullable
-        @Override
-        public String getPropertyName()
-        {
-            String name = getFlag();
-            if (!StringUtilsLabKey.isValidJavaIdentifier(name))
-            {
-                LOG.debug("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;
-        }
-    }
 }

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

@@ -41,7 +41,8 @@ public interface AppProps
 
     String SCOPE_SITE_SETTINGS = "SiteSettings";
 
-    String OPTIONAL_FEATURE = "experimentalFeature"; // Used for all optional features; "experimental" for historical reasons.
+    // Used for all optional features; "experimental" for historical reasons.
+    String OPTIONAL_FEATURE_PREFIX = "experimentalFeature.";
     String SCOPE_OPTIONAL_FEATURE = "ExperimentalFeature"; // Startup property prefix for all optional features; "Experimental" for historical reasons.
     String EXPERIMENTAL_NO_GUESTS = "disableGuestAccount";
     String EXPERIMENTAL_BLOCKER = "blockMaliciousClients";

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

@@ -75,7 +75,6 @@ class AppPropsImpl extends AbstractWriteableSettingsGroup implements AppProps
 
     static final String LOOK_AND_FEEL_REVISION = "logoRevision";
     static final String DEFAULT_LSID_AUTHORITY_PROP = "defaultLsidAuthority";
-    static final String OPTIONAL_FEATURE_PREFIX = OPTIONAL_FEATURE + ".";
     static final String ALLOW_LIST_DELIMITER = "\n";
 
     private static final String SITE_CONFIG_NAME = "SiteConfig";