From 5a1965920060208c8040b6ae6d362ed3763dd8a7 Mon Sep 17 00:00:00 2001 From: cnathe Date: Thu, 10 Jul 2025 15:48:56 -0500 Subject: [PATCH 01/10] Issue 53431: Data class with field name with special char doesn't round trip values through folder export/import --- .../org/labkey/experiment/samples/AbstractExpFolderWriter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java index 1fbf2724e44..1962f24ffc6 100644 --- a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java +++ b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java @@ -96,7 +96,7 @@ protected void writeTsv(TableInfo tinfo, Collection columns, SimpleF try (TSVGridWriter tsvWriter = new TSVGridWriter(factory)) { tsvWriter.setApplyFormats(false); - tsvWriter.setColumnHeaderType(ColumnHeaderType.FieldKey); + tsvWriter.setColumnHeaderType(ColumnHeaderType.ImportField); // Issue 53431 PrintWriter out = dir.getPrintWriter(baseName + ".tsv"); tsvWriter.write(out); } From 788d886592216d5047c48c04580bfbd32acd60d9 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 15 Jul 2025 10:41:25 -0500 Subject: [PATCH 02/10] A few more tests to use FieldInfo.random() --- .../tests/query/CrossFolderDataClassTest.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java b/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java index 65ca1a09580..13836c28796 100644 --- a/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java +++ b/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java @@ -8,6 +8,7 @@ import org.labkey.test.categories.Daily; import org.labkey.test.pages.query.ExecuteQueryPage; import org.labkey.test.params.FieldDefinition; +import org.labkey.test.params.FieldInfo; import org.labkey.test.params.experiment.DataClassDefinition; import org.labkey.test.util.exp.DataClassAPIHelper; @@ -54,15 +55,16 @@ private void doSetup() * Issue 45664: addresses the problem where DataClass metadata wasn't available in query when querying cross-folder */ @Test - public void testIssue454644() throws Exception + public void testIssue45664() throws Exception { String dataClass = "TopFolderDataClass"; var fields = Arrays.asList( - new FieldDefinition("intColumn", FieldDefinition.ColumnType.Integer), - new FieldDefinition("decimalColumn", FieldDefinition.ColumnType.Decimal), - new FieldDefinition("stringColumn", FieldDefinition.ColumnType.String), - new FieldDefinition("sampleDate", FieldDefinition.ColumnType.DateAndTime), - new FieldDefinition("boolColumn", FieldDefinition.ColumnType.Boolean)); + FieldInfo.random("intColumn", FieldDefinition.ColumnType.Integer).getFieldDefinition(), + FieldInfo.random("decimalColumn", FieldDefinition.ColumnType.Decimal).getFieldDefinition(), + FieldInfo.random("stringColumn", FieldDefinition.ColumnType.String).getFieldDefinition(), + FieldInfo.random("sampleDate", FieldDefinition.ColumnType.DateAndTime).getFieldDefinition(), + FieldInfo.random("boolColumn", FieldDefinition.ColumnType.Boolean).getFieldDefinition() + ); // make a dataclass in the top folder, give it some data DataClassDefinition testType = new DataClassDefinition(dataClass).setFields(fields); var dGen = DataClassAPIHelper.createEmptyDataClass(getProjectName(), testType); @@ -81,9 +83,9 @@ public void testIssue454644() throws Exception // now insert a record into the dataclass, in the subfolder subfolderQueryPage.getDataRegion().clickInsertNewRow() .setField("Name", "Jeff") - .setField("intColumn", "5") - .setField("decimalColumn", "6.7") - .setField("stringColumn", "hey") + .setField(testType.getFieldByNamePart("intColumn").getName(), "5") + .setField(testType.getFieldByNamePart("decimalColumn").getName(), "6.7") + .setField(testType.getFieldByNamePart("stringColumn").getName(), "hey") .submit(); // gather the data from the view; should only see Jeff From e9b900fd8fbfa5007cf8832b348b3b0350eab381 Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 16 Jul 2025 15:04:05 -0500 Subject: [PATCH 03/10] PropertyController.ValidateDomainFieldsAction and usage in TestDataGenerator.randomFieldName --- .../property/PropertyController.java | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java index c1ee2680a67..ae7dd01a725 100644 --- a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java +++ b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java @@ -501,6 +501,54 @@ public Object execute(DomainApiForm form, BindException errors) } } + @Marshal(Marshaller.Jackson) + @RequiresPermission(ReadPermission.class) + public static class ValidateDomainFieldsAction extends MutatingApiAction + { + @Override + protected ObjectMapper createRequestObjectMapper() + { + ObjectMapper mapper = JsonUtil.DEFAULT_MAPPER.copy(); + _propertyService.configureObjectMapper(mapper, null); + return mapper; + } + + @Override + public Object execute(DomainApiForm form, BindException errors) + { + GWTDomain domainDesign = form.getDomainDesign(); + String kindName = form.getKind() == null ? form.getDomainKind() : form.getKind(); + DomainKind kind = PropertyService.get().getDomainKindByName(kindName); + if (kind == null) + throw new IllegalArgumentException("No domain kind matches name '" + kindName + "'"); + String typeURI = "urn:lsid:labkey.com:" + kindName + ".Folder-1000.1001:1002"; // note using a fake lsid here, since not all domain kinds override generateDomainURI + Domain domain = PropertyService.get().createDomain(getContainer(), typeURI, "test"); + + ApiSimpleResponse resp = new ApiSimpleResponse(); + ValidationException results = DomainUtil.validateProperties(null, domainDesign, kind, null, getUser()); + if (!results.hasErrors()) + { + for (GWTPropertyDescriptor field : domainDesign.getFields()) + { + try + { + DomainProperty dp = DomainUtil.addProperty(domain, field, new HashMap<>(), new HashSet<>(), results); + OntologyManager.validatePropertyDescriptor(dp.getPropertyDescriptor()); + } + catch (ChangePropertyDescriptorException e) + { + results.addFieldError(field.getName(), e.getMessage()); + } + } + } + + resp.put("success", !results.hasErrors()); + if (results.hasErrors()) + resp.put("errors", results.getErrors()); + return resp; + } + } + @Marshal(Marshaller.Jackson) @RequiresPermission(ReadPermission.class) //Real permissions will be enforced later on by the DomainKind public static class SaveDomainAction extends MutatingApiAction From fe8ddc6611417615684e7ff5240b6cbdebba810e Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 16 Jul 2025 15:24:09 -0500 Subject: [PATCH 04/10] TestDataGenerator.randomFieldName to take DomainKind (defaults to SampleSet) --- .../test/tests/query/CrossFolderDataClassTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java b/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java index 13836c28796..ba58c16657f 100644 --- a/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java +++ b/query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java @@ -10,6 +10,7 @@ import org.labkey.test.params.FieldDefinition; import org.labkey.test.params.FieldInfo; import org.labkey.test.params.experiment.DataClassDefinition; +import org.labkey.test.util.DomainUtils; import org.labkey.test.util.exp.DataClassAPIHelper; import java.util.Arrays; @@ -59,11 +60,11 @@ public void testIssue45664() throws Exception { String dataClass = "TopFolderDataClass"; var fields = Arrays.asList( - FieldInfo.random("intColumn", FieldDefinition.ColumnType.Integer).getFieldDefinition(), - FieldInfo.random("decimalColumn", FieldDefinition.ColumnType.Decimal).getFieldDefinition(), - FieldInfo.random("stringColumn", FieldDefinition.ColumnType.String).getFieldDefinition(), - FieldInfo.random("sampleDate", FieldDefinition.ColumnType.DateAndTime).getFieldDefinition(), - FieldInfo.random("boolColumn", FieldDefinition.ColumnType.Boolean).getFieldDefinition() + FieldInfo.random("intColumn", FieldDefinition.ColumnType.Integer, DomainUtils.DomainKind.DataClass).getFieldDefinition(), + FieldInfo.random("decimalColumn", FieldDefinition.ColumnType.Decimal, DomainUtils.DomainKind.DataClass).getFieldDefinition(), + FieldInfo.random("stringColumn", FieldDefinition.ColumnType.String, DomainUtils.DomainKind.DataClass).getFieldDefinition(), + FieldInfo.random("sampleDate", FieldDefinition.ColumnType.DateAndTime, DomainUtils.DomainKind.DataClass).getFieldDefinition(), + FieldInfo.random("boolColumn", FieldDefinition.ColumnType.Boolean, DomainUtils.DomainKind.DataClass).getFieldDefinition() ); // make a dataclass in the top folder, give it some data DataClassDefinition testType = new DataClassDefinition(dataClass).setFields(fields); From 21094028db1dbfbac047273c19f4bf51964d7c57 Mon Sep 17 00:00:00 2001 From: cnathe Date: Thu, 17 Jul 2025 10:10:27 -0500 Subject: [PATCH 05/10] Add DomainKind.validateDomainName to consolidate some of the duplicate checks in various domainKinds --- .../labkey/api/exp/api/ExperimentService.java | 2 ++ .../api/exp/api/SampleTypeDomainKind.java | 32 +++++-------------- .../labkey/api/exp/api/SampleTypeService.java | 2 ++ .../labkey/api/exp/property/DomainKind.java | 6 ++++ .../experiment/api/DataClassDomainKind.java | 7 ++++ .../experiment/api/ExperimentServiceImpl.java | 3 +- .../experiment/api/SampleTypeServiceImpl.java | 3 +- .../org/labkey/list/model/ListDomainKind.java | 29 ++++++++--------- 8 files changed, 43 insertions(+), 41 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 5e369154827..4ce4dc646c5 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -285,6 +285,8 @@ ValidationException updateDataClass( @Nullable String auditUserComment ); + void validateDataClassName(@NotNull Container c, @NotNull User u, String name, boolean skipExisting); + /** * Get all DataClass definitions in the container. If includeOtherContainers is true, * a user must be provided to check for read permission of the containers in scope. diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index e3c55d3286c..c2a4d1fb42a 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -444,39 +444,23 @@ public NameExpressionValidationResult validateNameExpressions(SampleTypeDomainKi return errors; } + @Override + public void validateDomainName(Container container, User user, @Nullable Domain domain, String name) + { + SampleTypeService.get().validateSampleTypeName(container, user, name, domain != null); + } + @Override public void validateOptions(Container container, User user, SampleTypeDomainKindProperties options, String name, Domain domain, GWTDomain updatedDomainDesign) { super.validateOptions(container, user, options, name, domain, updatedDomainDesign); - // verify and NameExpression values - TableInfo materialSourceTI = ExperimentService.get().getTinfoSampleType(); - - boolean isUpdate = domain != null; - if (!isUpdate) - { - if (name == null) - { - throw new IllegalArgumentException("You must supply a name for the sample type."); - } - else - { - ExpSampleType st = SampleTypeService.get().getSampleType(container, user, name); - if (st != null) - throw new IllegalArgumentException("A Sample Type with that name already exists."); - } - } - - // verify the length of the Name - int nameMax = materialSourceTI.getColumn("Name").getScale(); - if (name != null && name.length() >= nameMax) - throw new IllegalArgumentException("Value for Name field may not exceed " + nameMax + " characters."); + validateDomainName(container, user, domain, name); if (options == null) - { return; - } + TableInfo materialSourceTI = ExperimentService.get().getTinfoSampleType(); int nameExpMax = materialSourceTI.getColumn("NameExpression").getScale(); if (StringUtils.isNotBlank(options.getNameExpression()) && options.getNameExpression().length() > nameExpMax) throw new IllegalArgumentException("Value for Name Expression field may not exceed " + nameExpMax + " characters."); diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 2d3bc26ecff..8fa9ebd9b06 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -219,6 +219,8 @@ default Map incrementSampleCounts(@Nullable Date counterDate) */ Function,Map> getSampleCountsFunction(@Nullable Date counterDate); + void validateSampleTypeName(Container container, User user, String name, boolean skipExistingCheck); + void deleteSampleType(int rowId, Container c, User user, @Nullable String auditUserComment) throws ExperimentException; // used by DomainKind.invalidate() diff --git a/api/src/org/labkey/api/exp/property/DomainKind.java b/api/src/org/labkey/api/exp/property/DomainKind.java index ad6cd4ef74e..508ef06f025 100644 --- a/api/src/org/labkey/api/exp/property/DomainKind.java +++ b/api/src/org/labkey/api/exp/property/DomainKind.java @@ -368,6 +368,12 @@ public UpdateableTableInfo.ObjectUriType getObjectUriColumn() return null; } + /** + * Overridable validity check for domain name. Base implementation does nothing. + */ + public void validateDomainName(Container container, User user, @Nullable Domain domain, String name) + {} + /** * Overridable validity check. Base only executes canCreateDefinition check. * NOTE: Due to historical limitations throws runtime exceptions instead of validation errors diff --git a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java index fc87dd45df5..7efec176896 100644 --- a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java +++ b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java @@ -368,6 +368,13 @@ public void validateOptions(Container container, User user, DataClassDomainKindP } + @Override + public void validateDomainName(Container container, User user, @Nullable Domain domain, String name) + { + // will skipExisting here, that check will be made again during createDataClass / updateDataClass + ExperimentService.get().validateDataClassName(container, user, name, true); + } + @Override public Domain createDomain(GWTDomain domain, DataClassDomainKindProperties options, Container container, User user, @Nullable TemplateInfo templateInfo, boolean forUpdate) { diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 0409a0d1fd3..6613ca20d06 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -8060,7 +8060,8 @@ public ValidationException updateDataClass(@NotNull Container c, @NotNull User u return errors; } - private void validateDataClassName(@NotNull Container c, @NotNull User u, String name, boolean skipExisting) throws IllegalArgumentException + @Override + public void validateDataClassName(@NotNull Container c, @NotNull User u, String name, boolean skipExisting) { if (name == null) throw new ApiUsageException("DataClass name is required."); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 10abe5ad1e6..03ccd6a1e5e 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -993,7 +993,8 @@ public Function,Map> getSampleCountsFunction(@Null }; } - private void validateSampleTypeName(Container container, User user, String name, boolean skipExistingCheck) + @Override + public void validateSampleTypeName(Container container, User user, String name, boolean skipExistingCheck) { if (name == null || StringUtils.isBlank(name)) throw new ApiUsageException("Sample Type name is required."); diff --git a/list/src/org/labkey/list/model/ListDomainKind.java b/list/src/org/labkey/list/model/ListDomainKind.java index 327dafecf5a..c04fb2c1640 100644 --- a/list/src/org/labkey/list/model/ListDomainKind.java +++ b/list/src/org/labkey/list/model/ListDomainKind.java @@ -358,17 +358,23 @@ public Class getTypeClass() } @Override - public Domain createDomain(GWTDomain domain, ListDomainKindProperties listProperties, Container container, User user, @Nullable TemplateInfo templateInfo, boolean forUpdate) + public void validateDomainName(Container container, User user, @Nullable Domain domain, String name) { - String name = StringUtils.trimToEmpty(domain.getName()); - String keyName = listProperties.getKeyName(); - if (StringUtils.isEmpty(name)) - throw new ApiUsageException("List name is required"); + throw new ApiUsageException("List name is required."); if (name.length() > MAX_NAME_LENGTH) - throw new ApiUsageException("List name cannot be longer than " + MAX_NAME_LENGTH + " characters"); - if (ListService.get().getList(container, name, true) != null) + throw new ApiUsageException("List name cannot be longer than " + MAX_NAME_LENGTH + " characters."); + if (ListService.get().getList(container, name, domain == null) != null) throw new ApiUsageException("The name '" + name + "' is already in use."); + } + + @Override + public Domain createDomain(GWTDomain domain, ListDomainKindProperties listProperties, Container container, User user, @Nullable TemplateInfo templateInfo, boolean forUpdate) + { + String name = StringUtils.trimToEmpty(domain.getName()); + validateDomainName(container, user, null, name); + + String keyName = listProperties.getKeyName(); if (StringUtils.isEmpty(keyName)) throw new ApiUsageException("List keyName is required"); @@ -497,14 +503,7 @@ else if (!key.getName().equalsIgnoreCase(newKey.getName())) boolean hasNameChange = !original.getName().equals(updatedName); if (hasNameChange) { - if (updatedName.length() > MAX_NAME_LENGTH) - { - return exception.addGlobalError("List name cannot be longer than " + MAX_NAME_LENGTH + " characters."); - } - else if (ListService.get().getList(container, updatedName, false) != null) - { - return exception.addGlobalError("The name '" + updatedName + "' is already in use."); - } + validateDomainName(container, user, domain, updatedName); changeDetails.append("The name of the list domain '" + original.getName() + "' was changed to '" + updatedName + "'."); } From a6580111b11dd70839dbc450bfb9aa26017e2f94 Mon Sep 17 00:00:00 2001 From: cnathe Date: Thu, 17 Jul 2025 10:19:42 -0500 Subject: [PATCH 06/10] Update PropertyController.ValidateDomainAndFieldNamesAction to also validate domain names based on the domain kind --- .../property/PropertyController.java | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java index ae7dd01a725..86b3be987dd 100644 --- a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java +++ b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java @@ -503,7 +503,7 @@ public Object execute(DomainApiForm form, BindException errors) @Marshal(Marshaller.Jackson) @RequiresPermission(ReadPermission.class) - public static class ValidateDomainFieldsAction extends MutatingApiAction + public static class ValidateDomainAndFieldNamesAction extends MutatingApiAction { @Override protected ObjectMapper createRequestObjectMapper() @@ -524,23 +524,44 @@ public Object execute(DomainApiForm form, BindException errors) String typeURI = "urn:lsid:labkey.com:" + kindName + ".Folder-1000.1001:1002"; // note using a fake lsid here, since not all domain kinds override generateDomainURI Domain domain = PropertyService.get().createDomain(getContainer(), typeURI, "test"); + boolean checkFieldNames = !domainDesign.getFields().isEmpty(); + boolean checkDomainName = domainDesign.getName() != null; + ApiSimpleResponse resp = new ApiSimpleResponse(); - ValidationException results = DomainUtil.validateProperties(null, domainDesign, kind, null, getUser()); - if (!results.hasErrors()) + ValidationException results = new ValidationException(); + if (checkFieldNames) { - for (GWTPropertyDescriptor field : domainDesign.getFields()) + results = DomainUtil.validateProperties(null, domainDesign, kind, null, getUser()); + if (!results.hasErrors()) { - try - { - DomainProperty dp = DomainUtil.addProperty(domain, field, new HashMap<>(), new HashSet<>(), results); - OntologyManager.validatePropertyDescriptor(dp.getPropertyDescriptor()); - } - catch (ChangePropertyDescriptorException e) + for (GWTPropertyDescriptor field : domainDesign.getFields()) { - results.addFieldError(field.getName(), e.getMessage()); + try + { + DomainProperty dp = DomainUtil.addProperty(domain, field, new HashMap<>(), new HashSet<>(), results); + OntologyManager.validatePropertyDescriptor(dp.getPropertyDescriptor()); + } + catch (ChangePropertyDescriptorException e) + { + results.addFieldError(field.getName(), e.getMessage()); + } } } } + if (checkDomainName) + { + try + { + kind.validateDomainName(getContainer(), getUser(), null, domainDesign.getName()); + String nameError = DomainUtil.validateDomainName(domainDesign.getName(), kindName, kind.supportsNamingPattern()); + if (nameError != null) + results.addGlobalError(nameError); + } + catch (Exception e) + { + results.addGlobalError(e.getMessage()); + } + } resp.put("success", !results.hasErrors()); if (results.hasErrors()) From e861de10cb9372c1a5a89766aebe5e73e233b00c Mon Sep 17 00:00:00 2001 From: cnathe Date: Thu, 17 Jul 2025 10:20:54 -0500 Subject: [PATCH 07/10] Update usages of randomFieldName to pass DomainKind --- .../src/org/labkey/test/tests/study/StudyDatasetsTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java index b5ec3451669..a3feb7bc3ee 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java @@ -33,9 +33,11 @@ import org.labkey.test.pages.ImportDataPage; import org.labkey.test.pages.TimeChartWizard; import org.labkey.test.pages.study.DatasetDesignerPage; +import org.labkey.test.params.FieldDefinition; import org.labkey.test.params.FieldInfo; import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.DataRegionTable; +import org.labkey.test.util.DomainUtils; import org.labkey.test.util.LogMethod; import org.labkey.test.util.LoggedParam; import org.labkey.test.util.PortalHelper; @@ -219,7 +221,7 @@ public void testDatasetRoundTripWithSpecialChars() // Issue 53431 { goToManageStudy(); String datasetName = "Issue 53431"; - FieldInfo fieldInfo = FieldInfo.random("test,./field"); + FieldInfo fieldInfo = FieldInfo.random("test,./field", FieldDefinition.ColumnType.String, DomainUtils.DomainKind.StudyDatasetVisit); DatasetDesignerPage definitionPage = _studyHelper.goToManageDatasets() .clickCreateNewDataset() .setName(datasetName); From 3943934a95097f1cc5250955861275fe92d43adb Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 18 Jul 2025 09:24:15 -0500 Subject: [PATCH 08/10] StudyDatasetsTest to use TestDataUtils.tsvStringFromRowMaps for dataset data import via textarea --- .../org/labkey/test/tests/study/StudyDatasetsTest.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java index a3feb7bc3ee..149aecbc520 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java @@ -42,6 +42,7 @@ import org.labkey.test.util.LoggedParam; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.TestDataGenerator; +import org.labkey.test.util.data.TestDataUtils; import org.openqa.selenium.WebElement; import java.io.File; @@ -228,7 +229,13 @@ public void testDatasetRoundTripWithSpecialChars() // Issue 53431 DomainFormPanel panel = definitionPage.getFieldsPanel(); panel.manuallyDefineFields(fieldInfo.getFieldDefinition()); definitionPage.clickSave(); - importDatasetData(datasetName, "mouseId\tsequenceNum\t\"" + fieldInfo.getName() + "\"\n", "a1\t1\ttest123", "All data"); + importDatasetData(datasetName, "", TestDataUtils.tsvStringFromRowMaps( + List.of(Map.of( + "mouseId", "a1", + "sequenceNum", "1", + fieldInfo.getName(), "test123" + )), List.of("mouseId", "sequenceNum", fieldInfo.getName()), true + ), "All data"); File exportedFolder = exportFolderAsZip(null, false, false, false, false); deleteStudy(); From 7df1cd3620ec3e541fa694d005d6d5a61e74a92b Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 18 Jul 2025 09:46:51 -0500 Subject: [PATCH 09/10] update Sample Type required name error text --- experiment/src/client/test/integration/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/experiment/src/client/test/integration/utils.ts b/experiment/src/client/test/integration/utils.ts index c3bbdf83b1b..c6ade5a19c4 100644 --- a/experiment/src/client/test/integration/utils.ts +++ b/experiment/src/client/test/integration/utils.ts @@ -370,8 +370,8 @@ const LEGAL_CHARSET = [' ', '+', '-', '_', '.', ':', '', '&', '(', ')', '/']; const alphaNumeric = ['a', 'A', '1', '0']; export async function checkDomainName(server: IntegrationTestServer, domainType: string, supportNameExpression: boolean, folderOptions: RequestOptions, userOptions: RequestOptions) { const badNames = { - '': domainType === 'SampleSet' ? 'You must supply a name for the sample type.' : `${domainType} name must not be blank.`, - ' ': domainType === 'SampleSet' ? 'You must supply a name for the sample type.' : `${domainType} name must not be blank.`, + '': domainType === 'SampleSet' ? 'Sample Type name is required.' : `${domainType} name must not be blank.`, + ' ': domainType === 'SampleSet' ? 'Sample Type name is required.' : `${domainType} name must not be blank.`, 'with\0nullCharacter': `Invalid ${domainType} name 'REPLACE'. ${domainType} name must contain only valid unicode characters.`, 'with\tnewLines': `Invalid ${domainType} name 'REPLACE'. ${domainType} name may not contain 'tab', 'new line', or 'return' characters.`, '.startWithDot': `Invalid ${domainType} name 'REPLACE'. ${domainType} name must start with a letter or a number.`, @@ -408,7 +408,7 @@ export async function checkDomainName(server: IntegrationTestServer, domainType: let dataTypeRowId = 0; if (domainType !== 'SampleSet') dataTypeRowId = await getDataClassRowIdByName(server, domainName, folderOptions); - const requireMsg = `${domainType} name must not be blank.` + const requireMsg = domainType == 'SampleSet' ? 'Sample Type name is required.' : `${domainType} name must not be blank.`; badNames[''] = requireMsg; badNames[' '] = requireMsg; for (let i = 0; i < badNameKeys.length; i++){ From 4058086a19298709ec98b2305fc4c54782e8c9ba Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 22 Jul 2025 09:20:43 -0500 Subject: [PATCH 10/10] CR feedback - use const for domain kind names in integration/utils.ts and continue error check in ValidateDomainAndFieldNamesAction --- .../src/client/test/integration/utils.ts | 18 ++++++++++-------- .../property/PropertyController.java | 19 ++++++++----------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/experiment/src/client/test/integration/utils.ts b/experiment/src/client/test/integration/utils.ts index c6ade5a19c4..25a526e4435 100644 --- a/experiment/src/client/test/integration/utils.ts +++ b/experiment/src/client/test/integration/utils.ts @@ -16,6 +16,8 @@ export const SOURCE_TYPE_NAME_1 = 'SourceType1'; export const SOURCE_TYPE_NAME_2 = 'SourceType2'; export const ATTACHMENT_FIELD_1_NAME = 'SourceFile1'; export const ATTACHMENT_FIELD_2_NAME = 'SourceFile2'; +const SAMPLE_TYPE_DOMAIN_KIND = 'SampleSet'; +const DATA_CLASS_DOMAIN_KIND = 'DataClass'; // TODO move getSourceDataByName to ExperimentCrudUtils export async function getSourceDataByName(server: IntegrationTestServer, sourceName: string, queryName: string, columns: string = 'Name, RowId', folderOptions: RequestOptions , userOptions: RequestOptions, debug?: boolean) : Promise { @@ -306,7 +308,7 @@ export async function initProject(server: IntegrationTestServer, projectName: st } async function verifyDomainCreateFailure(server: IntegrationTestServer, domainType: string, badDomainName: string, error: string, folderOptions: RequestOptions, userOptions: RequestOptions, domainFields?: any[]) { - const field : Record = domainType === 'SampleSet' ? { name: 'Name' } : { name: 'Prop' }; + const field : Record = domainType === SAMPLE_TYPE_DOMAIN_KIND ? { name: 'Name' } : { name: 'Prop' }; const fields = [field]; if (domainFields) fields.push(...domainFields); @@ -349,7 +351,7 @@ async function verifyDomainUpdateFailure(server: IntegrationTestServer, domainId async function verifyDomainCreateSuccess(server: IntegrationTestServer, domainType: string, domainName: string, folderOptions: RequestOptions, userOptions: RequestOptions) { let domainId, domainURI; - const field = domainType === 'SampleSet' ? { name: 'Name' } : { name: 'Prop' }; + const field = domainType === SAMPLE_TYPE_DOMAIN_KIND ? { name: 'Name' } : { name: 'Prop' }; await server.post('property', 'createDomain', { kind: domainType, domainDesign: { name: domainName, fields: [field] }, @@ -370,8 +372,8 @@ const LEGAL_CHARSET = [' ', '+', '-', '_', '.', ':', '', '&', '(', ')', '/']; const alphaNumeric = ['a', 'A', '1', '0']; export async function checkDomainName(server: IntegrationTestServer, domainType: string, supportNameExpression: boolean, folderOptions: RequestOptions, userOptions: RequestOptions) { const badNames = { - '': domainType === 'SampleSet' ? 'Sample Type name is required.' : `${domainType} name must not be blank.`, - ' ': domainType === 'SampleSet' ? 'Sample Type name is required.' : `${domainType} name must not be blank.`, + '': domainType === SAMPLE_TYPE_DOMAIN_KIND ? 'Sample Type name is required.' : `${domainType} name must not be blank.`, + ' ': domainType === SAMPLE_TYPE_DOMAIN_KIND ? 'Sample Type name is required.' : `${domainType} name must not be blank.`, 'with\0nullCharacter': `Invalid ${domainType} name 'REPLACE'. ${domainType} name must contain only valid unicode characters.`, 'with\tnewLines': `Invalid ${domainType} name 'REPLACE'. ${domainType} name may not contain 'tab', 'new line', or 'return' characters.`, '.startWithDot': `Invalid ${domainType} name 'REPLACE'. ${domainType} name must start with a letter or a number.`, @@ -406,9 +408,9 @@ export async function checkDomainName(server: IntegrationTestServer, domainType: const { domainId, domainURI } = await verifyDomainCreateSuccess(server, domainType, domainName, folderOptions, userOptions); let dataTypeRowId = 0; - if (domainType !== 'SampleSet') + if (domainType !== SAMPLE_TYPE_DOMAIN_KIND) dataTypeRowId = await getDataClassRowIdByName(server, domainName, folderOptions); - const requireMsg = domainType == 'SampleSet' ? 'Sample Type name is required.' : `${domainType} name must not be blank.`; + const requireMsg = domainType == SAMPLE_TYPE_DOMAIN_KIND ? 'Sample Type name is required.' : `${domainType} name must not be blank.`; badNames[''] = requireMsg; badNames[' '] = requireMsg; for (let i = 0; i < badNameKeys.length; i++){ @@ -447,7 +449,7 @@ export async function checkLackDesignerOrReaderPerm(server: IntegrationTestServe export async function verifyRequiredLineageInsertUpdate(server: IntegrationTestServer, isParentSample: boolean, isChildSample: boolean, topFolderOptions: RequestOptions, subfolder1Options: RequestOptions, designerReaderOptions: RequestOptions, readerUserOptions: RequestOptions, editorUserOptions: RequestOptions, adminUserOptions: RequestOptions) { const parentDataType = isParentSample ? "ParentSampleType" : "ParentDataType"; await server.post('property', 'createDomain', { - kind: isParentSample ? 'SampleSet' : 'DataClass', + kind: isParentSample ? SAMPLE_TYPE_DOMAIN_KIND : DATA_CLASS_DOMAIN_KIND, domainDesign: { name: parentDataType, fields: [{ name: isParentSample ? 'Name' : 'Prop' }] }, options: { name: parentDataType, @@ -470,7 +472,7 @@ export async function verifyRequiredLineageInsertUpdate(server: IntegrationTestS const parentInput = (isParentSample ? (useLowerCase ? 'materialInputs/' : 'MaterialInputs/') : (useLowerCase ? 'dataInputs/' : 'DataInputs/')) + parentDataType; console.log("Selected alias input type: " + parentInput); await server.post('property', 'createDomain', { - kind: isChildSample ? 'SampleSet' : 'DataClass', + kind: isChildSample ? SAMPLE_TYPE_DOMAIN_KIND : DATA_CLASS_DOMAIN_KIND, domainDesign: { name: dataType, fields: [{ name: isChildSample ? 'Name' : 'Prop' }]}, options: { name: dataType, diff --git a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java index 86b3be987dd..1b0324ecbbd 100644 --- a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java +++ b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java @@ -532,19 +532,16 @@ public Object execute(DomainApiForm form, BindException errors) if (checkFieldNames) { results = DomainUtil.validateProperties(null, domainDesign, kind, null, getUser()); - if (!results.hasErrors()) + for (GWTPropertyDescriptor field : domainDesign.getFields()) { - for (GWTPropertyDescriptor field : domainDesign.getFields()) + try { - try - { - DomainProperty dp = DomainUtil.addProperty(domain, field, new HashMap<>(), new HashSet<>(), results); - OntologyManager.validatePropertyDescriptor(dp.getPropertyDescriptor()); - } - catch (ChangePropertyDescriptorException e) - { - results.addFieldError(field.getName(), e.getMessage()); - } + DomainProperty dp = DomainUtil.addProperty(domain, field, new HashMap<>(), new HashSet<>(), results); + OntologyManager.validatePropertyDescriptor(dp.getPropertyDescriptor()); + } + catch (ChangePropertyDescriptorException e) + { + results.addFieldError(field.getName(), e.getMessage()); } } }