Skip to content

Commit f5edec4

Browse files
jimczijimczi
committed
After review
This commit moves the deprecation warning in order to ensure that all cases of empty contexts are handled. This change also deprecates the indexation of a context completion field without contexts. This is needed because these suggestions are not reachable anymore if querying without context is removed.
1 parent 0696ec4 commit f5edec4

File tree

5 files changed

+72
-17
lines changed

5 files changed

+72
-17
lines changed

‎docs/reference/search/suggesters/context-suggest.asciidoc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ PUT place_path_category
8484
NOTE: Adding context mappings increases the index size for completion field. The completion index
8585
is entirely heap resident, you can monitor the completion field index size using <<indices-stats>>.
8686

87+
NOTE: deprecated[7.0.0, Indexing a suggestion without context on a context enabled completion field is deprecated
88+
and will be removed in the next major release. If you want to index a suggestion that matches all contexts you should
89+
add a special context for it.]
90+
8791
[[suggester-context-category]]
8892
[float]
8993
==== Category Context

‎rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ setup:
337337
- match: { suggest.result.0.options.0.text: "foo" }
338338

339339
---
340-
"Querying without contexts is deprecated":
340+
"Indexing and Querying without contexts is deprecated":
341341
- skip:
342342
version: " - 6.99.99"
343343
reason: this feature was deprecated in 7.0
@@ -353,6 +353,21 @@ setup:
353353
input: "foo"
354354
contexts:
355355
color: "red"
356+
suggest_multi_contexts:
357+
input: "bar"
358+
contexts:
359+
color: "blue"
360+
361+
- do:
362+
warnings:
363+
- "The ability to index a suggestion with no context on a context enabled completion field is deprecated and will be removed in the next major release."
364+
index:
365+
index: test
366+
type: test
367+
id: 2
368+
body:
369+
suggest_context:
370+
input: "foo"
356371

357372
- do:
358373
indices.refresh: {}
@@ -369,3 +384,32 @@ setup:
369384
field: suggest_context
370385

371386
- length: { suggest.result: 1 }
387+
388+
- do:
389+
warnings:
390+
- "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release."
391+
search:
392+
body:
393+
suggest:
394+
result:
395+
text: "foo"
396+
completion:
397+
field: suggest_context
398+
contexts: {}
399+
400+
- length: { suggest.result: 1 }
401+
402+
- do:
403+
warnings:
404+
- "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release."
405+
search:
406+
body:
407+
suggest:
408+
result:
409+
text: "foo"
410+
completion:
411+
field: suggest_multi_contexts
412+
contexts:
413+
location: []
414+
415+
- length: { suggest.result: 1 }

‎server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@
8383
* for query-time filtering and boosting (see {@link ContextMappings}
8484
*/
8585
public class CompletionFieldMapper extends FieldMapper implements ArrayValueMapperParser {
86-
8786
public static final String CONTENT_TYPE = "completion";
8887

8988
public static class Defaults {

‎server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
import org.elasticsearch.common.bytes.BytesReference;
2525
import org.elasticsearch.common.io.stream.StreamInput;
2626
import org.elasticsearch.common.io.stream.StreamOutput;
27-
import org.elasticsearch.common.logging.DeprecationLogger;
28-
import org.elasticsearch.common.logging.Loggers;
2927
import org.elasticsearch.common.unit.Fuzziness;
3028
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
3129
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@@ -60,9 +58,6 @@
6058
*/
6159
public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSuggestionBuilder> {
6260

63-
private static final DeprecationLogger DEPRECATION_LOGGER =
64-
new DeprecationLogger(Loggers.getLogger(CompletionSuggestionBuilder.class));
65-
6661
private static final XContentType CONTEXT_BYTES_XCONTENT_TYPE = XContentType.JSON;
6762
static final String SUGGESTION_NAME = "completion";
6863
static final ParseField CONTEXTS_FIELD = new ParseField("contexts", "context");
@@ -304,19 +299,16 @@ public SuggestionContext build(QueryShardContext context) throws IOException {
304299
if (mappedFieldType == null || mappedFieldType instanceof CompletionFieldMapper.CompletionFieldType == false) {
305300
throw new IllegalArgumentException("Field [" + suggestionContext.getField() + "] is not a completion suggest field");
306301
}
307-
CompletionFieldMapper.CompletionFieldType type = (CompletionFieldMapper.CompletionFieldType) mappedFieldType;
308-
suggestionContext.setFieldType(type);
309-
if (type.hasContextMappings()) {
310-
if (contextBytes == null) {
311-
DEPRECATION_LOGGER.deprecated("The ability to query with no context on a context enabled completion field is deprecated " +
312-
"and will be removed in the next major release.");
313-
} else {
302+
if (mappedFieldType instanceof CompletionFieldMapper.CompletionFieldType) {
303+
CompletionFieldMapper.CompletionFieldType type = (CompletionFieldMapper.CompletionFieldType) mappedFieldType;
304+
suggestionContext.setFieldType(type);
305+
if (type.hasContextMappings() && contextBytes != null) {
314306
Map<String, List<ContextMapping.InternalQueryContext>> queryContexts = parseContextBytes(contextBytes,
315-
context.getXContentRegistry(), type.getContextMappings());
307+
context.getXContentRegistry(), type.getContextMappings());
316308
suggestionContext.setQueryContexts(queryContexts);
309+
} else if (contextBytes != null) {
310+
throw new IllegalArgumentException("suggester [" + type.name() + "] doesn't expect any context");
317311
}
318-
} else if (contextBytes != null) {
319-
throw new IllegalArgumentException("suggester [" + type.name() + "] doesn't expect any context");
320312
}
321313
assert suggestionContext.getFieldType() != null : "no completion field type set";
322314
return suggestionContext;

‎server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import org.apache.lucene.util.CharsRefBuilder;
2626
import org.elasticsearch.ElasticsearchParseException;
2727
import org.elasticsearch.Version;
28+
import org.elasticsearch.common.logging.DeprecationLogger;
29+
import org.elasticsearch.common.logging.Loggers;
2830
import org.elasticsearch.common.xcontent.ToXContent;
2931
import org.elasticsearch.common.xcontent.XContentBuilder;
3032
import org.elasticsearch.index.mapper.CompletionFieldMapper;
@@ -51,6 +53,10 @@
5153
* for a {@link CompletionFieldMapper}
5254
*/
5355
public class ContextMappings implements ToXContent {
56+
57+
private static final DeprecationLogger DEPRECATION_LOGGER =
58+
new DeprecationLogger(Loggers.getLogger(ContextMappings.class));
59+
5460
private final List<ContextMapping> contextMappings;
5561
private final Map<String, ContextMapping> contextNameMap;
5662

@@ -143,6 +149,10 @@ protected Iterable<CharSequence> contexts() {
143149
scratch.setLength(1);
144150
}
145151
}
152+
if (typedContexts.isEmpty()) {
153+
DEPRECATION_LOGGER.deprecated("The ability to index a suggestion with no context on a context enabled completion field" +
154+
" is deprecated and will be removed in the next major release.");
155+
}
146156
return typedContexts;
147157
}
148158
}
@@ -156,6 +166,7 @@ protected Iterable<CharSequence> contexts() {
156166
*/
157167
public ContextQuery toContextQuery(CompletionQuery query, Map<String, List<ContextMapping.InternalQueryContext>> queryContexts) {
158168
ContextQuery typedContextQuery = new ContextQuery(query);
169+
boolean hasContext = false;
159170
if (queryContexts.isEmpty() == false) {
160171
CharsRefBuilder scratch = new CharsRefBuilder();
161172
scratch.grow(1);
@@ -169,10 +180,15 @@ public ContextQuery toContextQuery(CompletionQuery query, Map<String, List<Conte
169180
scratch.append(context.context);
170181
typedContextQuery.addContext(scratch.toCharsRef(), context.boost, !context.isPrefix);
171182
scratch.setLength(1);
183+
hasContext = true;
172184
}
173185
}
174186
}
175187
}
188+
if (hasContext == false) {
189+
DEPRECATION_LOGGER.deprecated("The ability to query with no context on a context enabled completion field is deprecated " +
190+
"and will be removed in the next major release.");
191+
}
176192
return typedContextQuery;
177193
}
178194

0 commit comments

Comments
 (0)