GH Issue 1341 & 1493: Remove "Allow RowId to be accepted when merging" and "Use row-by-row update" experimental flags (#8162)

Author: XingY

api/src/org/labkey/api/exp/api/ExperimentService.java

@@ -131,8 +131,6 @@ public interface ExperimentService extends ExperimentRunTypeSource
 
     String EXPERIMENTAL_FEATURE_FROM_EXPANCESTORS = "org.labkey.api.exp.api.ExperimentService#FROM_EXPANCESTORS";
 
-    String EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE = "org.labkey.experiment.api.SampleTypeUpdateServiceDI#ALLOW_ROW_ID_SAMPLE_MERGE";
-
     int SIMPLE_PROTOCOL_FIRST_STEP_SEQUENCE = 1;
     int SIMPLE_PROTOCOL_CORE_STEP_SEQUENCE = 10;
     int SIMPLE_PROTOCOL_EXTRA_STEP_SEQUENCE = 15;

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

@@ -75,7 +75,6 @@ public interface QueryService
     String MAX_QUERY_SELECTION = "maxQuerySelection";
     String PRODUCT_FOLDERS_ENABLED = "isProductFoldersEnabled";
     String PRODUCT_FOLDERS_EXIST = "hasProductFolders";
-    String USE_ROW_BY_ROW_UPDATE = "useLegacyUpdateRows";
 
     String MODULE_QUERIES_DIRECTORY = "queries";
     Path MODULE_QUERIES_PATH = Path.parse(MODULE_QUERIES_DIRECTORY);

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

@@ -53,8 +53,6 @@
 import java.util.Map;
 import java.util.function.Supplier;
 
-import static org.junit.Assume.assumeFalse;
-import static org.labkey.api.query.QueryService.USE_ROW_BY_ROW_UPDATE;
 import static org.labkey.api.query.QueryUpdateService.ConfigParameters.PreferPKOverObjectUriAsKey;
 
 /**
@@ -107,9 +105,6 @@ private boolean shouldUpdateUsingDIB(Container container, List<Map<String, Objec
         if (oldKeys != null) // could be called by updateChangingKeys
             return false;
 
-        if (OptionalFeatureService.get().isFeatureEnabled(USE_ROW_BY_ROW_UPDATE))
-            return false;
-
         if (!supportUpdateUsingDIB())
             return false;
 
@@ -329,7 +324,6 @@ private static Map<String, Object> row(String text, int intNotNull)
         @Test
         public void testAfterInsertUpdateFiresOnce() throws Exception
         {
-            assumeFalse("Row-by-row update bypasses the DIB update path", OptionalFeatureService.get().isFeatureEnabled(USE_ROW_BY_ROW_UPDATE));
             CountingUpdateService qus = createService();
 
             BatchValidationException errors = new BatchValidationException();

experiment/src/client/test/integration/SampleTypeCrud.ispec.ts

@@ -1184,7 +1184,7 @@ describe('Amount/Unit CRUD', () => {
         errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\tUnits\n" + dataName + "\t-1.1\tkg", dataType, "MERGE", topFolderOptions, editorUserOptions);
         expect(errorMsg.text).toContain(NEGATIVE_ERROR);
 
-        // Using row-by-row
+        // Using partitioned data iterator
         await server.post('query', 'updateRows', {
             schemaName: 'samples',
             queryName: dataType,

experiment/src/org/labkey/experiment/ExperimentModule.java

@@ -276,8 +276,6 @@ protected void init()
                 "If a column name contains a \"__<unit>\" 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, true);
-        OptionalFeatureService.get().addExperimentalFeatureFlag(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE, "Allow RowId to be accepted when merging samples or data class data",
-                "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());

experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java

@@ -113,7 +113,6 @@
 import org.labkey.api.security.permissions.Permission;
 import org.labkey.api.security.permissions.ReadPermission;
 import org.labkey.api.security.permissions.UpdatePermission;
-import org.labkey.api.settings.OptionalFeatureService;
 import org.labkey.api.study.assay.FileLinkDisplayColumn;
 import org.labkey.api.usageMetrics.SimpleMetricsService;
 import org.labkey.api.util.CachingSupplier;
@@ -1027,9 +1026,7 @@ else if (isReservedHeader(name))
                         if (isUpdate)
                             continue;
 
-                        // While accepting RowId during merge is not our preferred behavior, we want to give users a way
-                        // to opt-in to the old behavior where RowId is accepted and ignored.
-                        if (isMerge && !OptionalFeatureService.get().isFeatureEnabled(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE))
+                        if (isMerge)
                         {
                             context.getErrors().addRowError(new ValidationException("RowId is not accepted when merging data. Specify only the data name instead.", Column.RowId.name()));
                             return null;

experiment/src/org/labkey/experiment/api/ImportAbortResourceTestCase.java

@@ -29,7 +29,6 @@
 import org.labkey.api.dataiterator.MapDataIterator;
 import org.labkey.api.dataiterator.WrapperDataIterator;
 import org.labkey.api.exp.api.ExpSampleType;
-import org.labkey.api.exp.api.ExperimentService;
 import org.labkey.api.exp.api.SampleTypeService;
 import org.labkey.api.exp.query.ExpSchema;
 import org.labkey.api.exp.query.SamplesSchema;
@@ -38,7 +37,6 @@
 import org.labkey.api.query.QueryUpdateService;
 import org.labkey.api.query.QueryUpdateService.InsertOption;
 import org.labkey.api.security.User;
-import org.labkey.api.settings.OptionalFeatureService;
 import org.labkey.api.util.JunitUtil;
 import org.labkey.api.util.TestContext;
 import org.labkey.experiment.ExpDataIterators;
@@ -59,26 +57,18 @@ public class ImportAbortResourceTestCase extends Assert
 {
     private static Container _c;
     private static User _user;
-    private static boolean _restoreAllowRowIdMerge;
 
     @BeforeClass
     public static void setUp()
     {
         JunitUtil.deleteTestContainer();
         _c = JunitUtil.getTestContainer();
         _user = TestContext.get().getUser();
-
-        // These tests drive the RowId-on-merge rejection, so the server-wide opt-out must be off.
-        _restoreAllowRowIdMerge = OptionalFeatureService.get().isFeatureEnabled(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE);
-        if (_restoreAllowRowIdMerge)
-            OptionalFeatureService.get().setFeatureEnabled(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE, false, _user);
     }
 
     @AfterClass
     public static void tearDown()
     {
-        if (_restoreAllowRowIdMerge)
-            OptionalFeatureService.get().setFeatureEnabled(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE, true, _user);
         JunitUtil.deleteTestContainer();
     }
 

experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java

@@ -103,7 +103,6 @@
 import org.labkey.api.security.permissions.MoveEntitiesPermission;
 import org.labkey.api.security.permissions.ReadPermission;
 import org.labkey.api.security.roles.ReaderRole;
-import org.labkey.api.settings.OptionalFeatureService;
 import org.labkey.api.study.publish.StudyPublishService;
 import org.labkey.api.usageMetrics.SimpleMetricsService;
 import org.labkey.api.util.GUID;
@@ -1332,9 +1331,7 @@ private DataIterator build(DataIterator di, DataIteratorContext context)
                         if (isUpdate)
                             continue;
 
-                        // While accepting RowId during merge is not our preferred behavior, we want to give users a way
-                        // to opt-in to the old behavior where RowId is accepted and ignored.
-                        if (isMerge && !OptionalFeatureService.get().isFeatureEnabled(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE))
+                        if (isMerge)
                         {
                             context.getErrors().addRowError(new ValidationException("RowId is not accepted when merging samples. Specify only the sample name instead.", RowId.name()));
                             return null;

query/src/org/labkey/query/QueryModule.java

@@ -147,8 +147,6 @@
 import java.util.Set;
 import java.util.function.Supplier;
 
-import static org.labkey.api.query.QueryService.USE_ROW_BY_ROW_UPDATE;
-
 public class QueryModule extends DefaultModule
 {
     public QueryModule()
@@ -237,8 +235,6 @@ public QuerySchema createSchema(DefaultSchema schema, Module module)
         DataViewService.get().registerProvider(QueryDataViewProvider.TYPE, new QueryDataViewProvider());
         DataViewService.get().registerProvider(InheritedQueryDataViewProvider.TYPE, new InheritedQueryDataViewProvider());
 
-        OptionalFeatureService.get().addExperimentalFeatureFlag(USE_ROW_BY_ROW_UPDATE, "Use row-by-row update",
-            "For Query.updateRows api, do row-by-row update, instead of using a prepared statement that updates rows in batches.", false);
         OptionalFeatureService.get().addExperimentalFeatureFlag(QueryServiceImpl.EXPERIMENTAL_PRODUCT_ALL_FOLDER_LOOKUPS, "Less restrictive product folder lookups",
             "Allow for lookup fields in product folders to query across all folders within the top-level folder.", false);
         OptionalFeatureService.get().addExperimentalFeatureFlag(QueryServiceImpl.EXPERIMENTAL_PRODUCT_PROJECT_DATA_LISTING_SCOPED, "Product folders display folder-specific data",