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

Using consistent collection of non displayable fields to filter against. #3585

Merged
merged 5 commits into from Mar 28, 2017

Conversation

Projects
None yet
2 participants
@dennisoelkers
Member

dennisoelkers commented Mar 7, 2017

Description

Motivation and Context

Before this change, the "source" field was suppressed from being
returned (and therefore) displayed in decorated results. This was caused
by some fields included in the RESERVED_FIELDS set, which were not
added back from the decorator processor before returning the list of fields included
in the search result.

This change could have extracted the method used in SearchResult.java
into a commonly used helper method used in both code points. This would
have involved a lot of casting/conversion between different
(ResultMessage/ResultMessageSummary) data types for each returned
message which is decorated, so the impact of deduplication would be
bigger than its benefits. Instead, I cleaned up and (hopefully) made the
sets of fields more expressive and easier to use.

Fixes #3584.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

dennisoelkers added some commits Mar 7, 2017

Using consistent collection of non displayable fields to filter against.
Before this change, the "source" field was suppressed from being
returned (and therefore) displayed in decorated results. This was caused
by some fields included in the `RESERVED_FIELDS` set, which were not
[added
back](https://github.com/Graylog2/graylog2-server/blob/master/graylog2-server/src/main/java/org/graylog2/indexer/results/SearchResult.java#L79-L84)
from the decorator processor before returning the list of fields included
in the search result
(https://github.com/Graylog2/graylog2-server/blob/master/graylog2-server/src/main/java/org/graylog2/decorators/DecoratorProcessorImpl.java#L95-L100)..

This change could have extracted the method used in `SearchResult.java`
into a commonly used helper method used in both code points. This would
have involved a lot of casting/conversion between different
(`ResultMessage`/`ResultMessageSummary`) data types for each returned
message which is decorated, so the impact of deduplication would be
bigger than its benefits. Instead, I cleaned up and (hopefully) made the
sets of fields more expressive and easier to use.

Fixes #3584.

@dennisoelkers dennisoelkers added the bug label Mar 7, 2017

@dennisoelkers dennisoelkers added this to the 2.3.0 milestone Mar 7, 2017

.addAll(GRAYLOG_FIELDS)
.addAll(ES_FIELDS)
.add(FIELD_STREAMS)
.add(FIELD_FULL_MESSAGE)

This comment has been minimized.

@joschi

joschi Mar 16, 2017

Contributor

Why is full_message in the set for "non-displayable" fields? Shouldn't it be possible to display it?

This comment has been minimized.

@dennisoelkers

dennisoelkers Mar 17, 2017

Member

For some reasons it was filtered out for the filtered fields returned by the search, also not displayed in the sidebar.

This comment has been minimized.

@joschi

joschi Mar 17, 2017

Contributor

What exactly do you mean with "not displayed in the sidebar"?

Graylog sidebar

This comment has been minimized.

@dennisoelkers

dennisoelkers Mar 28, 2017

Member

The reason why you are seeing the full_message field is because the result is decorated. This is actually part of the bug that is fixed in this PR.

@joschi

joschi approved these changes Mar 28, 2017

LGTM.

@joschi joschi merged commit a724a27 into master Mar 28, 2017

4 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 1473 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@joschi joschi deleted the issue-3584 branch Mar 28, 2017

dennisoelkers added a commit that referenced this pull request Mar 28, 2017

Using consistent collection of non displayable fields to filter again…
…st. (#3585)

Before this change, the "source" field was suppressed from being
returned (and therefore) displayed in decorated results. This was caused
by some fields included in the `RESERVED_FIELDS` set, which were not
added back from the decorator processor before returning the list of fields included
in the search result.

This change could have extracted the method used in `SearchResult.java`
into a commonly used helper method used in both code points. This would
have involved a lot of casting/conversion between different
(`ResultMessage`/`ResultMessageSummary`) data types for each returned
message which is decorated, so the impact of deduplication would be
bigger than its benefits. Instead, I cleaned up and (hopefully) made the
sets of fields more expressive and easier to use.

Fixes #3584
(cherry picked from commit a724a27)

joschi added a commit that referenced this pull request Mar 28, 2017

Using consistent collection of non displayable fields to filter again…
…st (#3668)

Before this change, the "source" field was suppressed from being
returned (and therefore) displayed in decorated results. This was caused
by some fields included in the `RESERVED_FIELDS` set, which were not
added back from the decorator processor before returning the list of fields included
in the search result.

This change could have extracted the method used in `SearchResult.java`
into a commonly used helper method used in both code points. This would
have involved a lot of casting/conversion between different
(`ResultMessage`/`ResultMessageSummary`) data types for each returned
message which is decorated, so the impact of deduplication would be
bigger than its benefits. Instead, I cleaned up and (hopefully) made the
sets of fields more expressive and easier to use.

Fixes #3584
Refs #3585
(cherry picked from commit a724a27)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment