Skip to content

Assay: update instead of overwrite domain fields #6905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-nicka merged 4 commits into from
Aug 12, 2025
Merged

Assay: update instead of overwrite domain fields #6905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af00b92
Assay: augment instead of overwrite domain fields
labkey-nicka Aug 4, 2025
9887ec9
Fix test case
labkey-nicka Aug 5, 2025
2e37610
Add test
labkey-nicka Aug 5, 2025
d08205f
Deep copy rather than update by reference
labkey-nicka Aug 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions api/gwtsrc/org/labkey/api/gwt/client/model/GWTPropertyDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,17 @@ public GWTPropertyDescriptor(String name, String rangeURI)

public GWTPropertyDescriptor(GWTPropertyDescriptor s)
{
setPropertyId(s.getPropertyId());
setPropertyURI(s.getPropertyURI());
this(s, false);
}

public GWTPropertyDescriptor(GWTPropertyDescriptor s, boolean isNew)
{
if (!isNew)
{
setPropertyId(s.getPropertyId());
setPropertyURI(s.getPropertyURI());
}

setContainer(s.getContainer());
setName(s.getName());
setDescription(s.getDescription());
Expand Down Expand Up @@ -148,7 +157,13 @@ public GWTPropertyDescriptor(GWTPropertyDescriptor s)

for (GWTPropertyValidator v : s.getPropertyValidators())
{
propertyValidators.add(new GWTPropertyValidator(v));
GWTPropertyValidator gpv = new GWTPropertyValidator(v);
if (isNew)
{
gpv.setRowId(0);
gpv.setNew(true);
}
propertyValidators.add(gpv);
}

for (GWTConditionalFormat f : s.getConditionalFormats())
Expand Down
9 changes: 7 additions & 2 deletions api/src/org/labkey/api/exp/property/DomainUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,14 +476,19 @@ public static GWTDomain<GWTPropertyDescriptor> getTemplateDomainForDomainKind(Do

public static GWTPropertyDescriptor getPropertyDescriptor(DomainProperty prop)
{
return getPropertyDescriptor(prop.getPropertyDescriptor());
return getPropertyDescriptor(prop, false);
}

public static GWTPropertyDescriptor getPropertyDescriptor(PropertyDescriptor prop)
{
return getPropertyDescriptor(prop, false);
}

public static GWTPropertyDescriptor getPropertyDescriptor(DomainProperty prop, boolean copy)
{
return getPropertyDescriptor(prop.getPropertyDescriptor(), copy);
}

public static GWTPropertyDescriptor getPropertyDescriptor(PropertyDescriptor prop, boolean copy)
{
GWTPropertyDescriptor gwtProp = new GWTPropertyDescriptor();
Expand Down Expand Up @@ -543,7 +548,7 @@ public static GWTPropertyDescriptor getPropertyDescriptor(PropertyDescriptor pro

Map<String, String> properties = new HashMap<>(pv.getProperties());
// add in the TextChoice validValues here so that the client side code doesn't have to do the same
// parsing of the validator expression (i.e. sorting, trimming, removing duplicates, etc.)
// parsing of the validator expression (i.e., sorting, trimming, removing duplicates, etc.)
if (PropertyValidatorType.TextChoice.equals(gpv.getType()))
{
List<String> validValues = PropertyService.get().getTextChoiceValidatorOptions(pv);
Expand Down
43 changes: 19 additions & 24 deletions assay/src/org/labkey/assay/AssayDomainServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.labkey.assay;

import org.apache.commons.beanutils.ConvertUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -160,13 +159,18 @@ private List<GWTDomain<GWTPropertyDescriptor>> getDomains(
gwtDomain.setDefaultValuesURL(setDefaultValuesAction.getLocalURIString());
gwtDomain.setProvisioned(domain.isProvisioned());

List<GWTPropertyDescriptor> gwtProps = new ArrayList<>();
Map<DomainProperty, Object> defaultValues = domainInfo.getValue();
List<GWTPropertyDescriptor> fields = new ArrayList<>();
Set<String> mandatoryPropertyDescriptors = new CaseInsensitiveHashSet(domain.getDomainKind().getMandatoryPropertyNames(domain));

Map<String, GWTPropertyDescriptor> domainFields = new HashMap<>();
for (GWTPropertyDescriptor field : gwtDomain.getFields())
domainFields.put(field.getName(), field);

for (DomainProperty prop : domain.getProperties())
{
GWTPropertyDescriptor gwtProp = getPropertyDescriptor(prop, copy);
GWTPropertyDescriptor domainField = domainFields.get(prop.getName());
GWTPropertyDescriptor gwtProp = new GWTPropertyDescriptor(domainField, copy);

if (gwtProp.getDefaultValueType() == null)
{
// Explicitly set these "special" properties NOT to remember the user's last entered
Expand All @@ -182,17 +186,17 @@ private List<GWTDomain<GWTPropertyDescriptor>> getDomains(
gwtProp.setDefaultValueType(gwtDomain.getDefaultDefaultValueType());
}

Object defaultValue = defaultValues.get(prop);
if (AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME.equals(gwtProp.getName()) && defaultValue instanceof String)
if (AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME.equals(gwtProp.getName()))
{
Container studyContainer = ContainerManager.getForId((String) defaultValue);
if (studyContainer != null)
gwtProp.setDefaultDisplayValue(studyContainer.getPath());
Object defaultValue = domainInfo.getValue().get(prop);
if (defaultValue instanceof String containerId)
{
Container studyContainer = ContainerManager.getForId(containerId);
if (studyContainer != null)
gwtProp.setDefaultDisplayValue(studyContainer.getPath());
}
}
else
gwtProp.setDefaultDisplayValue(DomainUtil.getFormattedDefaultValue(getUser(), prop, defaultValue));

gwtProp.setDefaultValue(ConvertUtils.convert(defaultValue));
if (provider.isMandatoryDomainProperty(domain, prop.getName()))
mandatoryPropertyDescriptors.add(prop.getName());

Expand All @@ -208,11 +212,11 @@ private List<GWTDomain<GWTPropertyDescriptor>> getDomains(
gwtProp.setFilterCriteria(FilterCriteria.toGWTFilterCriteria(fieldFilterCriteria));
}

gwtProps.add(gwtProp);
fields.add(gwtProp);
}

gwtProps.addAll(gwtDomain.getCalculatedFields());
gwtDomain.setFields(gwtProps);
fields.addAll(gwtDomain.getCalculatedFields());
gwtDomain.setFields(fields);
gwtDomain.setMandatoryFieldNames(mandatoryPropertyDescriptors);

if (isResultsDomain)
Expand Down Expand Up @@ -348,15 +352,6 @@ private GWTContainer convertToGWTContainer(Container c)
return new GWTContainer(c.getId(), c.getRowId(), c.getPath(), c.getName());
}

private GWTPropertyDescriptor getPropertyDescriptor(DomainProperty prop, boolean copy)
{
GWTPropertyDescriptor gwtProp = DomainUtil.getPropertyDescriptor(prop);
if (copy)
gwtProp.setPropertyId(0);

return gwtProp;
}

private void setPlateTemplateList(AssayProvider provider, GWTProtocol protocol)
{
if (provider instanceof PlateBasedAssayProvider)
Expand Down
76 changes: 59 additions & 17 deletions assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,37 +140,28 @@
assayTemplate.setEditableResults(true);
List<GWTDomain<GWTPropertyDescriptor>> domains = assayTemplate.getDomains();

// clear the batch domain fields
GWTDomain<GWTPropertyDescriptor> batchDomain = domains.stream().filter(d -> "Batch Fields".equals(d.getName())).findFirst().orElseThrow();
batchDomain.getFields(true).clear();

// clear the run domain fields
GWTDomain<GWTPropertyDescriptor> runDomain = domains.stream().filter(d -> "Run Fields".equals(d.getName())).findFirst().orElseThrow();
runDomain.getFields(true).clear();

// clear the result domain fields and add a sample lookup
GWTDomain<GWTPropertyDescriptor> resultDomain = domains.stream().filter(d -> "Data Fields".equals(d.getName())).findFirst().orElseThrow();
resultDomain.getFields(true).clear();
List<GWTPropertyDescriptor> runFields = new ArrayList<>();
List<GWTPropertyDescriptor> resultFields = new ArrayList<>();

if (editableRunsAndResults)
{
GWTPropertyDescriptor runProp = new GWTPropertyDescriptor("runProp", "int");
runDomain.getFields(true).add(runProp);
runFields.add(runProp);
GWTPropertyDescriptor resultProp = new GWTPropertyDescriptor("resultProp", "int");
resultDomain.getFields(true).add(resultProp);
resultFields.add(resultProp);
}

// Lookup to exp.Materials on run domain
GWTPropertyDescriptor runExpMaterialLookup = new GWTPropertyDescriptor("RunExpMaterialsLookup", "int");
runExpMaterialLookup.setLookupSchema(ExpSchema.SCHEMA_NAME);
runExpMaterialLookup.setLookupQuery(ExpSchema.TableType.Materials.name());
runDomain.getFields(true).add(runExpMaterialLookup);
runFields.add(runExpMaterialLookup);

// Lookup to exp.Materials on results domain
GWTPropertyDescriptor resultExpMaterialLookup = new GWTPropertyDescriptor("ResultExpMaterialsLookup", "int");
resultExpMaterialLookup.setLookupSchema(ExpSchema.SCHEMA_NAME);
resultExpMaterialLookup.setLookupQuery(ExpSchema.TableType.Materials.name());
resultDomain.getFields(true).add(resultExpMaterialLookup);
resultFields.add(resultExpMaterialLookup);

if (includeSampleTypeLookups)
{
Expand All @@ -180,15 +171,27 @@
GWTPropertyDescriptor sampleTypeLookup = new GWTPropertyDescriptor("SampleTypeLookup", "string");
sampleTypeLookup.setLookupSchema(SamplesSchema.SCHEMA_NAME);
sampleTypeLookup.setLookupQuery(sampleType.getName());
resultDomain.getFields(true).add(sampleTypeLookup);
resultFields.add(sampleTypeLookup);

// Lookup to exp.materials.<sample type name>
GWTPropertyDescriptor expMaterialsSampleTypeLookup = new GWTPropertyDescriptor("ExpMaterialSampleTypeLookup", "int");
expMaterialsSampleTypeLookup.setLookupSchema(ExpSchema.SCHEMA_EXP_MATERIALS.toString());
expMaterialsSampleTypeLookup.setLookupQuery(sampleType.getName());
resultDomain.getFields(true).add(expMaterialsSampleTypeLookup);
resultFields.add(expMaterialsSampleTypeLookup);
}

// clear the batch domain fields
GWTDomain<GWTPropertyDescriptor> batchDomain = domains.stream().filter(d -> "Batch Fields".equals(d.getName())).findFirst().orElseThrow();
batchDomain.setFields(Collections.emptyList());

// set the run domain fields
GWTDomain<GWTPropertyDescriptor> runDomain = domains.stream().filter(d -> "Run Fields".equals(d.getName())).findFirst().orElseThrow();
runDomain.setFields(runFields);

// set the result domain fields
GWTDomain<GWTPropertyDescriptor> resultDomain = domains.stream().filter(d -> "Data Fields".equals(d.getName())).findFirst().orElseThrow();
resultDomain.setFields(resultFields);

// create the assay
log.info("creating assay");
GWTProtocol savedAssayDesign = assayDomainService.saveChanges(assayTemplate, true);
Expand Down Expand Up @@ -705,4 +708,43 @@

assertEquals(0, updatedRun.getMaterialInputs().size());
}

@Test
public void testDomainFieldConfiguration() throws Exception
{
// Validates assay domain fields are configured as expected by DomainUtil and not overwritten
// Arrange
var sampleTypeName = "Does Not Exist";
var lookupName = "Field100 ABCDEFGHIJKLMNOPQRSTUVWXYZ%()=+-[]_|*`'\":;<>?!@#^AABBCCDDEEFFGGHHIIJJKKLLMMNNOOPPQQRRSSTTU)";
var assayPair = createAssay(context, true, true);
var domainService = new AssayDomainServiceImpl(context);
var gwtProtocol = domainService.getAssayDefinition(assayPair.second.getRowId(), false);

// Update the assay design to refer to a non-existent sample type
{
var resultDomain = getResultDomain(gwtProtocol);
var resultFields = resultDomain.getFields(true);

GWTPropertyDescriptor invalidSampleLookup = new GWTPropertyDescriptor("SampleTypeLookup", "string");
invalidSampleLookup.setLookupSchema(SamplesSchema.SCHEMA_NAME);
invalidSampleLookup.setLookupQuery(sampleTypeName);
invalidSampleLookup.setName(lookupName);
resultFields.add(invalidSampleLookup);
resultDomain.setFields(resultFields);
gwtProtocol = domainService.saveChanges(gwtProtocol, true);
}

// Act
var updatedResultsDomain = getResultDomain(gwtProtocol);

// Assert
var lookup = updatedResultsDomain.getFieldByName(lookupName);
assertFalse("Expected lookup to be marked as invalid since sample type does not exist by that name", lookup.getLookupIsValid());
assertEquals("Expected lookup query to point to non-existent sample type", sampleTypeName, lookup.getLookupQuery());
}

private GWTDomain<GWTPropertyDescriptor> getResultDomain(GWTProtocol protocol)
{
return protocol.getDomains().stream().filter(d -> "Data".equals(d.getQueryName())).findFirst().orElseThrow();
}
%>