TestDataGenerator randomFieldName and randomDomainName usage of ValidateDomainAndFieldNamesAction

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 <code>includeOtherContainers</code> is true,
      * a user must be provided to check for read permission of the containers in scope.

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.");

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

@@ -219,6 +219,8 @@ default Map<String, Long> incrementSampleCounts(@Nullable Date counterDate)
      */
     Function<Map<String,Long>,Map<String,Long>> 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()

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

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<any> {
@@ -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<string, string> = domainType === 'SampleSet' ? { name: 'Name' } : { name: 'Prop' };
+    const field : Record<string, string> = 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' ? '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 === 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} 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,

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<GWTPropertyDescriptor> domain, DataClassDomainKindProperties options, Container container, User user, @Nullable TemplateInfo templateInfo, boolean forUpdate)
     {

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.");

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

@@ -993,7 +993,8 @@ public Function<Map<String,Long>,Map<String,Long>> 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.");

experiment/src/org/labkey/experiment/controllers/property/PropertyController.java

@@ -501,6 +501,72 @@ public Object execute(DomainApiForm form, BindException errors)
         }
     }
 
+    @Marshal(Marshaller.Jackson)
+    @RequiresPermission(ReadPermission.class)
+    public static class ValidateDomainAndFieldNamesAction extends MutatingApiAction<DomainApiForm>
+    {
+        @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<GWTPropertyDescriptor> 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");
+
+            boolean checkFieldNames = !domainDesign.getFields().isEmpty();
+            boolean checkDomainName = domainDesign.getName() != null;
+
+            ApiSimpleResponse resp = new ApiSimpleResponse();
+            ValidationException results = new ValidationException();
+            if (checkFieldNames)
+            {
+                results = DomainUtil.validateProperties(null, domainDesign, kind, null, getUser());
+                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());
+                    }
+                }
+            }
+            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())
+                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<DomainApiForm>

list/src/org/labkey/list/model/ListDomainKind.java

@@ -358,17 +358,23 @@ public Class<? extends ListDomainKindProperties> 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 + "'.");
             }
 

query/test/src/org/labkey/test/tests/query/CrossFolderDataClassTest.java

@@ -8,7 +8,9 @@
 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.DomainUtils;
 import org.labkey.test.util.exp.DataClassAPIHelper;
 
 import java.util.Arrays;
@@ -54,15 +56,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, 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);
         var dGen = DataClassAPIHelper.createEmptyDataClass(getProjectName(), testType);
@@ -81,9 +84,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