Skip to content

Add validation to feature set usage name#55350

Merged
jasontedor merged 1 commit intoelastic:masterfrom
jasontedor:x-pack-feature-set-usage-name-validation
Apr 16, 2020
Merged

Add validation to feature set usage name#55350
jasontedor merged 1 commit intoelastic:masterfrom
jasontedor:x-pack-feature-set-usage-name-validation

Conversation

@jasontedor
Copy link
Copy Markdown
Member

We do not validate the name is not null, and not empty. Even though it never should be, we had a build failure where it appears that somehow this did happen. We add some validation here, in case this really is happening, we will have a more clear indication where this is coming from, and of course, validation that name fits the implicit assumptions that it is not null and not empty.

We do not validate the name is not null, and not empty. Even though it
never should be, we had a build failure where it appears that somehow
this did happen. We add some validation here, in case this really is
happening, we will have a more clear indication where this is coming
from, and of course, validation that name fits the implicit assumptions
that it is not null and not empty.
@jasontedor jasontedor added >non-issue :Core/Infra/Core Core issues without another label v8.0.0 v7.8.0 labels Apr 16, 2020
@jasontedor jasontedor requested a review from rjernst April 16, 2020 21:12
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


public Usage(String name, boolean available, boolean enabled) {
Objects.requireNonNull(name);
if (name.isEmpty()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically isBlank() would be better incase it was just whitespace, but It's fine either way as this is very unlikely since this is not user input

@jasontedor jasontedor merged commit 3220e98 into elastic:master Apr 16, 2020
jasontedor added a commit that referenced this pull request Apr 16, 2020
We do not validate the name is not null, and not empty. Even though it
never should be, we had a build failure where it appears that somehow
this did happen. We add some validation here, in case this really is
happening, we will have a more clear indication where this is coming
from, and of course, validation that name fits the implicit assumptions
that it is not null and not empty.
@jasontedor jasontedor deleted the x-pack-feature-set-usage-name-validation branch April 16, 2020 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants