diff --git a/graylog2-server/src/main/java/org/graylog2/decorators/DecoratorProcessorImpl.java b/graylog2-server/src/main/java/org/graylog2/decorators/DecoratorProcessorImpl.java index 4e984e9d021b..b4441e322166 100644 --- a/graylog2-server/src/main/java/org/graylog2/decorators/DecoratorProcessorImpl.java +++ b/graylog2-server/src/main/java/org/graylog2/decorators/DecoratorProcessorImpl.java @@ -95,7 +95,7 @@ private String getMessageKey(ResultMessageSummary messageSummary) { private Set extractFields(List messages) { return messages.stream() .flatMap(message -> message.message().keySet().stream()) - .filter(field -> !Message.RESERVED_FIELDS.contains(field)) + .filter(field -> !Message.FILTERED_FIELDS.contains(field)) .collect(Collectors.toSet()); } diff --git a/graylog2-server/src/main/java/org/graylog2/indexer/results/SearchResult.java b/graylog2-server/src/main/java/org/graylog2/indexer/results/SearchResult.java index 48368aa76918..34abcb2dfaad 100644 --- a/graylog2-server/src/main/java/org/graylog2/indexer/results/SearchResult.java +++ b/graylog2-server/src/main/java/org/graylog2/indexer/results/SearchResult.java @@ -16,6 +16,7 @@ */ package org.graylog2.indexer.results; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Sets; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.unit.TimeValue; @@ -23,7 +24,6 @@ import org.graylog2.indexer.ranges.IndexRange; import org.graylog2.plugin.Message; -import java.util.Iterator; import java.util.List; import java.util.Set; @@ -45,45 +45,31 @@ public SearchResult(SearchHits searchHits, Set usedIndices, String o this.totalResults = searchHits.getTotalHits(); this.usedIndices = usedIndices; } - + public long getTotalResults() { return totalResults; } - + public List getResults() { return results; } - + public Set getFields() { return fields; } - private Set extractFields(List hits) { + @VisibleForTesting + Set extractFields(List hits) { Set filteredFields = Sets.newHashSet(); - Set allFields = Sets.newHashSet(); - - Iterator i = hits.iterator(); - while(i.hasNext()) { - final Message message = i.next().getMessage(); - allFields.addAll(message.getFieldNames()); + hits.forEach(hit -> { + final Message message = hit.getMessage(); for (String field : message.getFieldNames()) { - if (!Message.RESERVED_FIELDS.contains(field)) { + if (!Message.FILTERED_FIELDS.contains(field)) { filteredFields.add(field); } } - } - - // Because some fields actually make sense in this result and some don't. - // TODO: This is super awkward. First we do not include RESERVED_FIELDS, then we add some back... - if (allFields.contains("message")) { - filteredFields.add("message"); - } - if (allFields.contains("source")) { - filteredFields.add("source"); - } - filteredFields.remove("streams"); - filteredFields.remove("full_message"); + }); return filteredFields; } diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/Message.java b/graylog2-server/src/main/java/org/graylog2/plugin/Message.java index 6d3b68bb49a0..8de4c99a98b6 100644 --- a/graylog2-server/src/main/java/org/graylog2/plugin/Message.java +++ b/graylog2-server/src/main/java/org/graylog2/plugin/Message.java @@ -68,49 +68,56 @@ public class Message implements Messages { private static final Pattern VALID_KEY_CHARS = Pattern.compile("^[\\w\\.\\-@]*$"); private static final char KEY_REPLACEMENT_CHAR = '_'; - public static final ImmutableSet RESERVED_FIELDS = ImmutableSet.of( - // ElasticSearch fields. - FIELD_ID, - "_ttl", - "_source", - "_all", - "_index", - "_type", - "_score", - - // Our reserved fields. - FIELD_MESSAGE, - FIELD_SOURCE, - FIELD_TIMESTAMP, - "gl2_source_node", - "gl2_source_input", - "gl2_source_collector", - "gl2_source_collector_input", - "gl2_remote_ip", - "gl2_remote_port", - "gl2_remote_hostname", - // TODO Due to be removed in Graylog 3.x - "gl2_source_radio", - "gl2_source_radio_input" + private static final ImmutableSet GRAYLOG_FIELDS = ImmutableSet.of( + "gl2_source_node", + "gl2_source_input", + // TODO Due to be removed in Graylog 3.x + "gl2_source_radio", + "gl2_source_radio_input", + + "gl2_source_collector", + "gl2_source_collector_input", + "gl2_remote_ip", + "gl2_remote_port", + "gl2_remote_hostname" ); - public static final ImmutableSet RESERVED_SETTABLE_FIELDS = ImmutableSet.of( - FIELD_MESSAGE, - FIELD_SOURCE, - FIELD_TIMESTAMP, - "gl2_source_node", - "gl2_source_input", - "gl2_source_radio", - "gl2_source_radio_input", - "gl2_source_collector", - "gl2_source_collector_input", - "gl2_remote_ip", - "gl2_remote_port", - "gl2_remote_hostname" + private static final ImmutableSet CORE_MESSAGE_FIELDS = ImmutableSet.of( + FIELD_MESSAGE, + FIELD_SOURCE, + FIELD_TIMESTAMP ); + private static final ImmutableSet ES_FIELDS = ImmutableSet.of( + // ElasticSearch fields. + FIELD_ID, + "_ttl", + "_source", + "_all", + "_index", + "_type", + "_score" + ); + + public static final ImmutableSet RESERVED_SETTABLE_FIELDS = new ImmutableSet.Builder() + .addAll(GRAYLOG_FIELDS) + .addAll(CORE_MESSAGE_FIELDS) + .build(); + + public static final ImmutableSet RESERVED_FIELDS = new ImmutableSet.Builder() + .addAll(RESERVED_SETTABLE_FIELDS) + .addAll(ES_FIELDS) + .build(); + + public static final ImmutableSet FILTERED_FIELDS = new ImmutableSet.Builder() + .addAll(GRAYLOG_FIELDS) + .addAll(ES_FIELDS) + .add(FIELD_STREAMS) + .add(FIELD_FULL_MESSAGE) + .build(); + private static final ImmutableSet REQUIRED_FIELDS = ImmutableSet.of( - FIELD_MESSAGE, FIELD_ID + FIELD_MESSAGE, FIELD_ID ); public static final Function ID_FUNCTION = new MessageIdFunction(); @@ -198,7 +205,7 @@ public Map toElasticSearchObject(@Nonnull final Meter invalidTim obj.put(newKey, entry.getValue()); } else { LOG.warn("Keys must not contain a \".\" character! Ignoring field \"{}\"=\"{}\" in message [{}] - Unable to replace \".\" with a \"{}\" because of key conflict: \"{}\"=\"{}\"", - key, entry.getValue(), getId(), KEY_REPLACEMENT_CHAR, newKey, obj.get(newKey)); + key, entry.getValue(), getId(), KEY_REPLACEMENT_CHAR, newKey, obj.get(newKey)); LOG.debug("Full message with \".\" in message key: {}", this); } } else { @@ -207,7 +214,7 @@ public Map toElasticSearchObject(@Nonnull final Meter invalidTim // Deliberate warning duplicates because the key with the "." might be transformed before reaching // the duplicate original key with a "_". Otherwise we would silently overwrite the transformed key. LOG.warn("Keys must not contain a \".\" character! Ignoring field \"{}\"=\"{}\" in message [{}] - Unable to replace \".\" with a \"{}\" because of key conflict: \"{}\"=\"{}\"", - newKey, fields.get(newKey), getId(), KEY_REPLACEMENT_CHAR, key, entry.getValue()); + newKey, fields.get(newKey), getId(), KEY_REPLACEMENT_CHAR, key, entry.getValue()); LOG.debug("Full message with \".\" in message key: {}", this); } obj.put(key, entry.getValue()); diff --git a/graylog2-server/src/test/java/org/graylog2/indexer/results/SearchResultTest.java b/graylog2-server/src/test/java/org/graylog2/indexer/results/SearchResultTest.java new file mode 100644 index 000000000000..ccccd57813f7 --- /dev/null +++ b/graylog2-server/src/test/java/org/graylog2/indexer/results/SearchResultTest.java @@ -0,0 +1,91 @@ +/** + * This file is part of Graylog. + * + * Graylog is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Graylog is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Graylog. If not, see . + */ +package org.graylog2.indexer.results; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import org.elasticsearch.search.SearchHits; +import org.graylog2.plugin.Message; +import org.junit.Before; +import org.junit.Test; + +import java.util.Collections; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SearchResultTest { + private SearchResult searchResult; + + @Before + public void setUp() throws Exception { + final SearchHits searchHits = mock(SearchHits.class); + when(searchHits.iterator()).thenReturn(Collections.emptyIterator()); + this.searchResult = new SearchResult(searchHits, null, null, null, null); + } + + @Test + public void extractFieldsForEmptyResult() throws Exception { + final Set result = searchResult.extractFields(Collections.emptyList()); + + assertThat(result) + .isNotNull() + .isEmpty(); + } + + @Test + public void extractFieldsForTwoMessagesContainingDifferentFields() throws Exception { + final ResultMessage r1 = mock(ResultMessage.class); + final Message m1 = mock(Message.class); + when(m1.getFieldNames()).thenReturn(ImmutableSet.of( + "message", + "source", + "timestamp", + "http_response", + "gl2_source_node", + "_index" + )); + when(r1.getMessage()).thenReturn(m1); + + final ResultMessage r2 = mock(ResultMessage.class); + final Message m2 = mock(Message.class); + when(m2.getFieldNames()).thenReturn(ImmutableSet.of( + "message", + "source", + "timestamp", + "took_ms", + "gl2_source_collector" + )); + when(r2.getMessage()).thenReturn(m2); + + final Set result = searchResult.extractFields(ImmutableList.of(r1, r2)); + + assertThat(result) + .isNotNull() + .isNotEmpty() + .hasSize(5) + .containsExactlyInAnyOrder( + "message", + "source", + "timestamp", + "http_response", + "took_ms" + ); + } +}