Issue 52060: Do trimming of domain names earlier in the process so validation catches reserved words #6319
Conversation
|
|
||
| if (domain.getName() != null) | ||
| { | ||
| String domainNameError = validateDomainName(domain.getName(), kind.getKindName(), kind.supportsNamingPattern()); | ||
| String domainNameError = validateDomainName(StringUtils.trimToEmpty(domain.getName()), kind.getKindName(), kind.supportsNamingPattern()); |
There was a problem hiding this comment.
I always find this createDomain interface odd, in that the domain can already have name, but then an explicit but optional domainName is passed in. If the domain itself's name contains leading/trailing whitespace, but domainName is null, then the space is saved. Perhaps we should be another domain.setName(StringUtils.trimToNull(domain.getName)) here
There was a problem hiding this comment.
Updated in both cases to use trimToEmpty.
| @@ -767,7 +767,7 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT | |||
| return validationException; | |||
| } | |||
|
|
|||
| String updatedName = update.getName(); | |||
| String updatedName = StringUtils.trimToEmpty(update.getName()); | |||
There was a problem hiding this comment.
This change could be a little problematic. For legacy domains with names that contains illegal characters or patterns, we do still allow them to be updated as long as there is no domain name change. Now if the legacy domain name happened to have leading/trialing white space and also illegal pattern, then this domain can no longer be updated without having to be renamed.
There was a problem hiding this comment.
Yeah, I can see that. Since it seems unlikely that the untrimmed name will exist in any case and there is a workaround, I'm leaning toward leaving in this change, though so we'll have less chance of having untrimmed names saved. If anyone sees this as overly problematic, though, I can probably be convinced otherwise.
@cnathe Thoughts on this?
There was a problem hiding this comment.
After further testing, I've moved this trim inside the if (in both cases).
|
|
||
| if (original.getName() != null && update.getName() != null && !original.getName().equals(update.getName())) | ||
| String trimmedName = StringUtils.trimToNull(update.getName()); | ||
| if (original.getName() != null && trimmedName != null && !original.getName().equals(trimmedName)) |
There was a problem hiding this comment.
Same comment as above regarding legacy domain name support.
|
Should the component version be bumped to bring the space warning to lks? |
|
… with leading or trailing spaces
Rationale
Issue 52060 - LKSM: Spaces at the end of source type names are problematic
Related Pull Requests
Changes