diff --git a/api/src/org/labkey/api/settings/OptionalFeatureFlag.java b/api/src/org/labkey/api/settings/OptionalFeatureFlag.java index 5cabb76c0a1..45790e997d1 100644 --- a/api/src/org/labkey/api/settings/OptionalFeatureFlag.java +++ b/api/src/org/labkey/api/settings/OptionalFeatureFlag.java @@ -12,6 +12,7 @@ public class OptionalFeatureFlag implements Comparable, 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; @@ -19,10 +20,15 @@ public class OptionalFeatureFlag implements Comparable, Sta 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; @@ -30,6 +36,21 @@ public OptionalFeatureFlag(String flag, String title, String description, boolea _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; } } diff --git a/api/src/org/labkey/api/settings/OptionalFeatureService.java b/api/src/org/labkey/api/settings/OptionalFeatureService.java index 4244f5c5713..1ccb0fb1b18 100644 --- a/api/src/org/labkey/api/settings/OptionalFeatureService.java +++ b/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); diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 676ce04da99..4f1ceb3ae22 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -529,19 +529,20 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) } OptionalFeatureService.get().addExperimentalFeatureFlag(NotificationMenuView.EXPERIMENTAL_NOTIFICATION_MENU, "Notifications Menu", - "Notifications 'inbox' count display in the header bar with click to show the notifications panel of unread notifications.", false); + "Notifications 'inbox' count display in the header bar with click to show the notifications panel of unread notifications.", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(DataColumn.EXPERIMENTAL_USE_QUERYSELECT_COMPONENT, "Use QuerySelect for row insert/update form", "This feature will switch the query based select inputs on the row insert/update form to use the React QuerySelect" + "component. This will allow for a user to view the first 100 options in the select but then use type ahead" + - "search to find the other select values.", false); + "search to find the other select values.", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(SQLFragment.FEATUREFLAG_DISABLE_STRICT_CHECKS, "Disable SQLFragment strict checks", - "SQLFragment now has very strict usage validation, these checks may cause errors in code that has not been updated. Turn on this feature to disable checks.", false); + "SQLFragment now has very strict usage validation, these checks may cause errors in code that has not been updated. Turn on this feature to disable checks.", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(LoginController.FEATUREFLAG_DISABLE_LOGIN_XFRAME, "Disable Login X-FRAME-OPTIONS=DENY", - "By default LabKey disables all framing of login related actions. Disabling this feature will revert to using the standard site settings.", false); + "By default LabKey disables all framing of login related actions. Disabling this feature will revert to using the standard site settings.", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(PageTemplate.EXPERIMENTAL_SHORT_CIRCUIT_ROBOTS, "Short-circuit robots", "Save resources by not rendering pages marked as 'noindex' for robots. This is experimental as not all robots are search engines.", - false); + false, + true); OptionalFeatureService.get().addExperimentalFeatureFlag(AppProps.REJECT_CONTROLLER_FIRST_URLS, "Reject controller-first URLs", "Require standard path-first URLs.", diff --git a/devtools/src/org/labkey/devtools/DevtoolsModule.java b/devtools/src/org/labkey/devtools/DevtoolsModule.java index d391f62baa0..b918e6f01a7 100644 --- a/devtools/src/org/labkey/devtools/DevtoolsModule.java +++ b/devtools/src/org/labkey/devtools/DevtoolsModule.java @@ -21,7 +21,6 @@ import org.labkey.api.module.CodeOnlyModule; import org.labkey.api.module.ModuleContext; import org.labkey.api.security.AuthenticationManager; -import org.labkey.api.settings.OptionalFeatureFlag; import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.util.JspTestCase; import org.labkey.api.view.WebPartFactory; @@ -62,10 +61,10 @@ protected void init() addController("testsso", TestSsoController.class); AuthenticationManager.registerProvider(new TestSsoProvider()); - OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(Domain.EXPERIMENTAL_FUZZ_STORAGE_NAME, + OptionalFeatureService.get().addExperimentalFeatureFlag(Domain.EXPERIMENTAL_FUZZ_STORAGE_NAME, "'fuzz' name of database columns used to back domain properties", "This is dev/test feature and not intended for any production usage.", - false, false, OptionalFeatureService.FeatureType.Experimental)); + false, true); } @Override diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 5d452027778..e96b25ce651 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -262,7 +262,7 @@ protected void init() ExperimentService.get().registerNameExpressionType("dataclass", "exp", "DataClass", "nameexpression"); OptionalFeatureService.get().addExperimentalFeatureFlag(AppProps.EXPERIMENTAL_RESOLVE_PROPERTY_URI_COLUMNS, "Resolve property URIs as columns on experiment tables", - "If a column is not found on an experiment table, attempt to resolve the column name as a Property URI and add it as a property column", false); + "If a column is not found on an experiment table, attempt to resolve the column name as a Property URI and add it as a property column", false, true); if (CoreSchema.getInstance().getSqlDialect().isSqlServer()) { OptionalFeatureService.get().addExperimentalFeatureFlag(NameGenerator.EXPERIMENTAL_WITH_COUNTER, "Use strict incremental withCounter and rootSampleCount expression", @@ -282,9 +282,9 @@ protected void init() OptionalFeatureService.get().addExperimentalFeatureFlag(AppProps.QUANTITY_COLUMN_SUFFIX_TESTING, "Quantity column suffix testing", "If a column name contains a \"__\" suffix, this feature allows for testing it as a Quantity display column", false); OptionalFeatureService.get().addExperimentalFeatureFlag(ExperimentService.EXPERIMENTAL_FEATURE_FROM_EXPANCESTORS, "SQL syntax: 'FROM EXPANCESTORS()'", - "Support for querying lineage of experiment objects", false); + "Support for querying lineage of experiment objects", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(SampleTypeUpdateServiceDI.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_SAMPLE_MERGE, "Allow RowId to be accepted when merging samples", - "If the incoming data includes a RowId column we will allow the column but ignore it's values.", false); + "If the incoming data includes a RowId column we will allow the column but ignore it's values.", false, true); RoleManager.registerPermission(new DesignVocabularyPermission(), true); RoleManager.registerRole(new SampleTypeDesignerRole()); diff --git a/query/src/org/labkey/query/QueryModule.java b/query/src/org/labkey/query/QueryModule.java index 0630786a1a2..888f6ebf63c 100644 --- a/query/src/org/labkey/query/QueryModule.java +++ b/query/src/org/labkey/query/QueryModule.java @@ -237,7 +237,7 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) DataViewService.get().registerProvider(InheritedQueryDataViewProvider.TYPE, new InheritedQueryDataViewProvider()); OptionalFeatureService.get().addExperimentalFeatureFlag(QueryView.EXPERIMENTAL_GENERIC_DETAILS_URL, "Generic [details] link in grids/queries", - "This feature will turn on generating a generic [details] URL link in most grids.", false); + "This feature will turn on generating a generic [details] URL link in most grids.", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(QueryServiceImpl.EXPERIMENTAL_LAST_MODIFIED, "Include Last-Modified header on query metadata requests", "For schema, query, and view metadata requests include a Last-Modified header such that the browser can cache the response. " + "The metadata is invalidated when performing actions such as creating a new List or modifying the columns on a custom view", false);