Skip to content

Conversation

@williamrandolph
Copy link
Contributor

There was a mysterious failure in an upgrade test. Examining the logs, I found a NullPointerException that wasn't revealed in the test failure message. Fixing the NPE fixed the test.

Should the AliasMetadata#isHidden method be nullable? I can't remember why it was written that way, but I can dig if there are any concerns about this fix.

Fixes #81411

@williamrandolph williamrandolph added :Core/Infra/Core Core issues without another label v8.2.0 v8.1.1 v8.0.2 labels Mar 8, 2022
@williamrandolph williamrandolph requested a review from grcevski March 8, 2022 22:02
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Mar 8, 2022
@elasticmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @williamrandolph, I've created a changelog YAML for you.

Copy link
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.

I don't know why AliasMetadata allows isHidden to be null, but this change does look like it should protect from null here.

.anyMatch(a -> Objects.nonNull(a.isHidden()) && a.isHidden() == false)) {
for (AliasMetadata aliasMetadata : indexMetadata.getAliases().values()) {
if (aliasMetadata.isHidden() == false) {
if (Objects.nonNull(aliasMetadata.isHidden()) && aliasMetadata.isHidden() == false) {
Copy link
Member

Choose a reason for hiding this comment

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

another way to write this, without a compound expression, would be:

if (Boolean.FALSE.equals(aliasMetadata.isHidden())) {

Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM!

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.1
8.0

williamrandolph added a commit to williamrandolph/elasticsearch that referenced this pull request Mar 9, 2022
…alias handling (elastic#84780)

* Fix an NPE in hidden alias logic
* Update docs/changelog/84780.yaml
* Simplify conditional
elasticsearchmachine pushed a commit that referenced this pull request Mar 9, 2022
…alias handling (#84780) (#84834)

* Fix an NPE in hidden alias logic
* Update docs/changelog/84780.yaml
* Simplify conditional
elasticsearchmachine pushed a commit that referenced this pull request Mar 9, 2022
…alias handling (#84780) (#84833)

* Fix an NPE in hidden alias logic
* Update docs/changelog/84780.yaml
* Simplify conditional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team v8.0.2 v8.1.1 v8.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] FullClusterRestartIT testSecurityNativeRealm failing

5 participants