Skip to content

Commit 55ab015

Browse files
jpountzjpountz
committed
Handle the removal of basic models be, d and p, and after effect no.
1 parent cfa8c35 commit 55ab015

File tree

2 files changed

+129
-11
lines changed

2 files changed

+129
-11
lines changed

‎server/src/main/java/org/elasticsearch/index/similarity/SimilarityProviders.java‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,10 @@
2424
import org.apache.lucene.search.similarities.AfterEffectL;
2525
import org.apache.lucene.search.similarities.BM25Similarity;
2626
import org.apache.lucene.search.similarities.BasicModel;
27-
import org.apache.lucene.search.similarities.BasicModelBE;
28-
import org.apache.lucene.search.similarities.BasicModelD;
2927
import org.apache.lucene.search.similarities.BasicModelG;
3028
import org.apache.lucene.search.similarities.BasicModelIF;
3129
import org.apache.lucene.search.similarities.BasicModelIn;
3230
import org.apache.lucene.search.similarities.BasicModelIne;
33-
import org.apache.lucene.search.similarities.BasicModelP;
3431
import org.apache.lucene.search.similarities.BooleanSimilarity;
3532
import org.apache.lucene.search.similarities.ClassicSimilarity;
3633
import org.apache.lucene.search.similarities.DFISimilarity;
@@ -74,24 +71,35 @@ private SimilarityProviders() {} // no instantiation
7471
static final String DISCOUNT_OVERLAPS = "discount_overlaps";
7572

7673
private static final Map<String, BasicModel> BASIC_MODELS;
74+
private static final Map<String, String> LEGACY_BASIC_MODELS;
7775
private static final Map<String, AfterEffect> AFTER_EFFECTS;
76+
private static final Map<String, String> LEGACY_AFTER_EFFECTS;
7877

7978
static {
8079
Map<String, BasicModel> models = new HashMap<>();
81-
models.put("be", new BasicModelBE());
82-
models.put("d", new BasicModelD());
8380
models.put("g", new BasicModelG());
8481
models.put("if", new BasicModelIF());
8582
models.put("in", new BasicModelIn());
8683
models.put("ine", new BasicModelIne());
87-
models.put("p", new BasicModelP());
8884
BASIC_MODELS = unmodifiableMap(models);
8985

86+
Map<String, String> legacyModels = new HashMap<>();
87+
// TODO: be and g and both based on the bose-einstein model.
88+
// Is there a better replacement for d and p which use the binomial model?
89+
legacyModels.put("be", "g");
90+
legacyModels.put("d", "ine");
91+
legacyModels.put("p", "ine");
92+
LEGACY_BASIC_MODELS = unmodifiableMap(legacyModels);
93+
9094
Map<String, AfterEffect> effects = new HashMap<>();
91-
effects.put("no", new AfterEffect.NoAfterEffect());
9295
effects.put("b", new AfterEffectB());
9396
effects.put("l", new AfterEffectL());
9497
AFTER_EFFECTS = unmodifiableMap(effects);
98+
99+
Map<String, String> legacyEffects = new HashMap<>();
100+
// l is simpler than b, so this should be a better replacement for "no"
101+
legacyEffects.put("no", "l");
102+
LEGACY_AFTER_EFFECTS = unmodifiableMap(legacyEffects);
95103
}
96104

97105
private static final Map<String, Independence> INDEPENDENCE_MEASURES;
@@ -124,9 +132,23 @@ private SimilarityProviders() {} // no instantiation
124132
* @param settings Settings to parse
125133
* @return {@link BasicModel} referred to in the Settings
126134
*/
127-
private static BasicModel parseBasicModel(Settings settings) {
135+
private static BasicModel parseBasicModel(Version indexCreatedVersion, Settings settings) {
128136
String basicModel = settings.get("basic_model");
129137
BasicModel model = BASIC_MODELS.get(basicModel);
138+
139+
if (model == null) {
140+
String replacement = LEGACY_BASIC_MODELS.get(basicModel);
141+
if (replacement != null) {
142+
if (indexCreatedVersion.onOrAfter(Version.V_7_0_0_alpha1)) {
143+
throw new IllegalArgumentException("Basic model [" + basicModel + "] isn't supported anymore, please use another model.");
144+
} else {
145+
DEPRECATION_LOGGER.deprecated("Basic model [" + basicModel + "] isn't supported anymore and has arbitrarily been replaced with [" + replacement + "].");
146+
model = BASIC_MODELS.get(replacement);
147+
assert model != null;
148+
}
149+
}
150+
}
151+
130152
if (model == null) {
131153
throw new IllegalArgumentException("Unsupported BasicModel [" + basicModel + "], expected one of " + BASIC_MODELS.keySet());
132154
}
@@ -139,9 +161,23 @@ private static BasicModel parseBasicModel(Settings settings) {
139161
* @param settings Settings to parse
140162
* @return {@link AfterEffect} referred to in the Settings
141163
*/
142-
private static AfterEffect parseAfterEffect(Settings settings) {
164+
private static AfterEffect parseAfterEffect(Version indexCreatedVersion, Settings settings) {
143165
String afterEffect = settings.get("after_effect");
144166
AfterEffect effect = AFTER_EFFECTS.get(afterEffect);
167+
168+
if (effect == null) {
169+
String replacement = LEGACY_AFTER_EFFECTS.get(afterEffect);
170+
if (replacement != null) {
171+
if (indexCreatedVersion.onOrAfter(Version.V_7_0_0_alpha1)) {
172+
throw new IllegalArgumentException("After effect [" + afterEffect + "] isn't supported anymore, please use another effect.");
173+
} else {
174+
DEPRECATION_LOGGER.deprecated("After effect [" + afterEffect + "] isn't supported anymore and has arbitrarily been replaced with [" + replacement + "].");
175+
effect = AFTER_EFFECTS.get(replacement);
176+
assert effect != null;
177+
}
178+
}
179+
}
180+
145181
if (effect == null) {
146182
throw new IllegalArgumentException("Unsupported AfterEffect [" + afterEffect + "], expected one of " + AFTER_EFFECTS.keySet());
147183
}
@@ -263,8 +299,8 @@ public static DFRSimilarity createDfrSimilarity(Settings settings, Version index
263299

264300

265301
return new DFRSimilarity(
266-
parseBasicModel(settings),
267-
parseAfterEffect(settings),
302+
parseBasicModel(indexCreatedVersion, settings),
303+
parseAfterEffect(indexCreatedVersion, settings),
268304
parseNormalization(settings));
269305
}
270306

‎server/src/test/java/org/elasticsearch/index/similarity/SimilarityServiceTests.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,21 @@
1818
*/
1919
package org.elasticsearch.index.similarity;
2020

21+
import org.apache.lucene.search.similarities.AfterEffectB;
22+
import org.apache.lucene.search.similarities.AfterEffectL;
2123
import org.apache.lucene.search.similarities.BM25Similarity;
24+
import org.apache.lucene.search.similarities.BasicModelG;
25+
import org.apache.lucene.search.similarities.BasicModelIne;
2226
import org.apache.lucene.search.similarities.BooleanSimilarity;
27+
import org.apache.lucene.search.similarities.DFRSimilarity;
28+
import org.apache.lucene.search.similarities.Similarity;
29+
import org.elasticsearch.Version;
30+
import org.elasticsearch.cluster.metadata.IndexMetaData;
2331
import org.elasticsearch.common.settings.Settings;
2432
import org.elasticsearch.index.IndexSettings;
2533
import org.elasticsearch.test.ESTestCase;
2634
import org.elasticsearch.test.IndexSettingsModule;
35+
import org.hamcrest.Matchers;
2736

2837
import java.util.Collections;
2938

@@ -56,4 +65,77 @@ public void testOverrideDefaultSimilarity() {
5665
SimilarityService service = new SimilarityService(indexSettings, null, Collections.emptyMap());
5766
assertTrue(service.getDefaultSimilarity() instanceof BooleanSimilarity);
5867
}
68+
69+
public void testDeprecatedDFRSimilarities() {
70+
Settings settings = Settings.builder()
71+
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_6_4_0)
72+
73+
.put("index.similarity.my_sim1.type", "dfr")
74+
.put("index.similarity.my_sim1.model", "d")
75+
.put("index.similarity.my_sim1.normalization", "h2")
76+
.put("index.similarity.my_sim1.after_effect", "no")
77+
78+
.put("index.similarity.my_sim2.type", "dfr")
79+
.put("index.similarity.my_sim2.model", "p")
80+
.put("index.similarity.my_sim2.normalization", "h2")
81+
.put("index.similarity.my_sim2.after_effect", "l")
82+
83+
.put("index.similarity.my_sim2.type", "dfr")
84+
.put("index.similarity.my_sim2.model", "be")
85+
.put("index.similarity.my_sim2.normalization", "h2")
86+
.put("index.similarity.my_sim2.after_effect", "b")
87+
88+
.build();
89+
90+
IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", settings);
91+
SimilarityService service = new SimilarityService(indexSettings, null, Collections.emptyMap());
92+
93+
Similarity sim = service.getSimilarity("my_sim1").get();
94+
assertThat(sim, Matchers.instanceOf(DFRSimilarity.class));
95+
DFRSimilarity dfrSim = (DFRSimilarity) sim;
96+
assertThat(dfrSim.getBasicModel(), Matchers.instanceOf(BasicModelIne.class));
97+
assertThat(dfrSim.getAfterEffect(), Matchers.instanceOf(AfterEffectL.class));
98+
99+
sim = service.getSimilarity("my_sim2").get();
100+
assertThat(sim, Matchers.instanceOf(DFRSimilarity.class));
101+
dfrSim = (DFRSimilarity) sim;
102+
assertThat(dfrSim.getBasicModel(), Matchers.instanceOf(BasicModelIne.class));
103+
assertThat(dfrSim.getAfterEffect(), Matchers.instanceOf(AfterEffectL.class));
104+
105+
sim = service.getSimilarity("my_sim3").get();
106+
assertThat(sim, Matchers.instanceOf(DFRSimilarity.class));
107+
dfrSim = (DFRSimilarity) sim;
108+
assertThat(dfrSim.getBasicModel(), Matchers.instanceOf(BasicModelG.class));
109+
assertThat(dfrSim.getAfterEffect(), Matchers.instanceOf(AfterEffectB.class));
110+
111+
assertWarnings(
112+
"Basic model [d] isn't supported anymore and has arbitrarily been replaced with [ine].",
113+
"Basic model [p] isn't supported anymore and has arbitrarily been replaced with [ine].",
114+
"Basic model [be] isn't supported anymore and has arbitrarily been replaced with [g].",
115+
"After effect [no] isn't supported anymore and has arbitrarily been replaced with [l].");
116+
}
117+
118+
public void testRejectUnsupportedDFRSimilarities() {
119+
Settings settings = Settings.builder()
120+
.put("index.similarity.my_sim1.type", "dfr")
121+
.put("index.similarity.my_sim1.model", "d")
122+
.put("index.similarity.my_sim1.normalization", "h2")
123+
.put("index.similarity.my_sim1.after_effect", "l")
124+
.build();
125+
IndexSettings indexSettings1 = IndexSettingsModule.newIndexSettings("test", settings);
126+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
127+
() -> new SimilarityService(indexSettings1, null, Collections.emptyMap()));
128+
assertEquals("Basic model [d] isn't supported anymore, please use another model.", e.getMessage());
129+
130+
settings = Settings.builder()
131+
.put("index.similarity.my_sim1.type", "dfr")
132+
.put("index.similarity.my_sim1.model", "g")
133+
.put("index.similarity.my_sim1.normalization", "h2")
134+
.put("index.similarity.my_sim1.after_effect", "no")
135+
.build();
136+
IndexSettings indexSettings2 = IndexSettingsModule.newIndexSettings("test", settings);
137+
e = expectThrows(IllegalArgumentException.class,
138+
() -> new SimilarityService(indexSettings2, null, Collections.emptyMap()));
139+
assertEquals("After effect [no] isn't supported anymore, please use another effect.", e.getMessage());
140+
}
59141
}

0 commit comments

Comments
 (0)