Skip to content
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.

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

Improve error msg when a field name contains only white spaces #27709

Merged
merged 2 commits into from
Dec 8, 2017

Conversation

olcbean
Copy link
Contributor

@olcbean olcbean commented Dec 7, 2017

Till now if a field name is empty or contains only white spaces, the error msg is object field starting or ending with a [.] makes object resolution ambiguous.

This PR explicitly check if the field name contains only white spaces and result in the error object field cannot contain only white spaces.

Fixes #27701

CC @danielmitterdorfer @dakrone

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cbuescher
Copy link
Member

@elasticmachine test this please

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

I left two comments about the messages

// check if the field name contains only white spaces
if (Strings.isEmpty(part) == false) {
throw new IllegalArgumentException(
"object field cannot contain only white spaces: ['" + fullFieldPath + "']");
Copy link
Member

Choose a reason for hiding this comment

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

this should be "white spaces" -> "whitespace"

() -> client().prepareIndex("idx", "type").setSource(bytes, XContentType.JSON).get());

assertThat(emptyFieldNameException.getMessage(), containsString(
"object field cannot contain only white spaces: ['top.aoeu. ']"));
Copy link
Member

Choose a reason for hiding this comment

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

"white spaces" -> "whitespace"

@olcbean olcbean force-pushed the field_name_error_msg branch from 0029e4a to 92811d5 Compare December 8, 2017 18:29
@olcbean
Copy link
Contributor Author

olcbean commented Dec 8, 2017

@dakrone thanks! I just modified the msg.

@dakrone dakrone merged commit f50f99e into elastic:master Dec 8, 2017
@dakrone
Copy link
Member

dakrone commented Dec 8, 2017

Merged, thanks @olcbean!

dakrone pushed a commit that referenced this pull request Dec 8, 2017
* Explicitly check if a field name contains only
white spaces

* "white spaces" changed to "whitespace"
dakrone pushed a commit that referenced this pull request Dec 8, 2017
* Explicitly check if a field name contains only
white spaces

* "white spaces" changed to "whitespace"
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Dec 9, 2017
* master:
  Fix index with unknown setting test
  Remove internal channel tracking in transports (elastic#27711)
  Improve error msg when a field name contains only white spaces (elastic#27709)
  Do not open indices with broken settings
  Set ACK timeout on indices service test
  Implement byte array reusage in `NioTransport` (elastic#27696)
  [TEST] Remove leftover ES temp directories before Vagrant tests (elastic#27722)
  Cleanup split strings by comma method
  Remove unused import from AliasResolveRoutingIT
  Add read timeouts to http module (elastic#27713)
  Fix routing with leading or trailing whitespace
  remove await fix from FullClusterRestartIT.testRecovery
  Add missing 's' to tmpdir name (elastic#27721)
  [Issue-27716]: CONTRIBUTING.md IntelliJ configurations settings are confusing. (elastic#27717)
  [TEST] Now actually wait for merges
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants