Add DomainUtil.getNamesAndLabels utility method for use in reserving variants of column names #7060
Add DomainUtil.getNamesAndLabels utility method for use in reserving variants of column names #7060labkey-susanh merged 15 commits intodevelopfrom
DomainUtil.getNamesAndLabels utility method for use in reserving variants of column names #7060Conversation
…into fb_reservedFields
There was a problem hiding this comment.
Pull Request Overview
This PR adds utility methods DomainUtil.getNamesAndLabels and DomainUtil.getNameAndLabels to consistently reserve both the name and label variants of column names across various domain implementations. This ensures consistency when resolving columns during import operations where users can provide either the field name or its label.
Key changes include:
- Added new utility methods to
DomainUtilfor generating name and label variants - Updated numerous domain classes to use these utility methods instead of manual field name handling
- Standardized reserved field collections to use
CaseInsensitiveHashSetand made many static constants
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| api/src/org/labkey/api/exp/property/DomainUtil.java | Added getNameAndLabels and getNamesAndLabels utility methods |
| Multiple domain kind files | Updated to use new utility methods and consistent collection handling |
| Multiple files with getReservedPropertyNames | Added @NotNull annotations to method signatures |
Comments suppressed due to low confidence (1)
filecontent/src/org/labkey/filecontent/FilePropertiesDomainKind.java:1
- This method override has been removed but it's unclear if this change is intentional or related to the PR's purpose of adding name/label utilities. If this removal is unintentional, the method should be restored.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
XingY
left a comment
XingY
left a comment
There was a problem hiding this comment.
Could the getNamesAndLabels step be done in the centralized util instead?
| RESERVED_NAMES = new CaseInsensitiveHashSet(DomainUtil.getNamesAndLabels( | ||
| BASE_PROPERTIES.stream().map(PropertyStorageSpec::getName).collect(Collectors.toSet())) | ||
| ); | ||
| RESERVED_NAMES.addAll(DomainUtil.getNamesAndLabels(Arrays.stream(ExpSampleTypeTable.Column.values()).map(Enum::name).toList())); |
There was a problem hiding this comment.
Instead of each domain pass in NamesAndLabels as reservedNames, could we instead handle the getNamesAndLabels check in DomainUtil in a centralized place?
There was a problem hiding this comment.
I think getNamesAndLabels does what you're asking for if used differently. I've adjusted usages so we aren't constructing as many sets from sets.
Or were you thinking that DomainKind.getReservedPropertyNames would call getNamesAndLabels? I think we want to keep the sets returned from getReservedPropertyNames static where we can so we aren't always constructing new sets.
| } | ||
| } | ||
| )); | ||
| Set<String> set = new CaseInsensitiveHashSet(); |
There was a problem hiding this comment.
If this is done in DomainUtil then this change is not needed.
Rationale
Because we generally allow users to provide either the name of a field or its label during import, we should consistently be reserving both the name and the label for each domain so there will not be ambiguity when trying to resolve a column during import. This PR gets us closer to consistency by adding a utility method that will create the set of reservable strings given a field's name and then uses that in various domains, also making more of the reserved field sets static constants.
Related Pull Requests
DomainUtil.getNamesAndLabelsutility method for use in reserving variants of column names commonAssays#930Changes
DomainUtil.getNamesAndLabelsDomainUtil.getNameAndLabelsgetReservedPropertyNamesas@NotNullTasks 📍
Manual TestingNeeds Automation