Skip to content

Commit

Permalink
SolrRequest.getParams never null; and define mutability
Browse files Browse the repository at this point in the history
Add SolrParams.of()
  • Loading branch information
dsmiley committed Jan 27, 2025
1 parent b1fe883 commit bfdec90
Show file tree
Hide file tree
Showing 39 changed files with 193 additions and 169 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ private void commitOnLeader(String leaderBaseUrl, String coreName)
throws SolrServerException, IOException {
try (SolrClient client = recoverySolrClientBuilder(leaderBaseUrl, coreName).build()) {
UpdateRequest ureq = new UpdateRequest();
ureq.setParams(new ModifiableSolrParams());
// ureq.getParams().set(DistributedUpdateProcessor.COMMIT_END_POINT, true);
// ureq.getParams().set(UpdateParams.OPEN_SEARCHER, onlyLeaderIndexes);
// Why do we need to open searcher if "onlyLeaderIndexes"?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ static UpdateResponse softCommit(String baseUrl, String coreName)
.withSocketTimeout(120000, TimeUnit.MILLISECONDS)
.build()) {
UpdateRequest ureq = new UpdateRequest();
ureq.setParams(new ModifiableSolrParams());
ureq.setAction(AbstractUpdateRequest.ACTION.COMMIT, false, true, true);
return ureq.process(client);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
}
try {
SolrParams params = req.getParams();
assert params != null;
UpdateRequestProcessorChain processorChain = req.getCore().getUpdateProcessorChain(params);

UpdateRequestProcessor processor = processorChain.createProcessor(req, rsp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ static Long getCheckSum(Checksum checksum, Path f) {
private volatile IndexFetcher currentIndexFetcher;

public IndexFetchResult doFetch(SolrParams solrParams, boolean forceReplication) {
String leaderUrl = solrParams == null ? null : solrParams.get(LEADER_URL, null);
String leaderUrl = solrParams.get(LEADER_URL, null);
if (!indexFetchLock.tryLock()) return IndexFetchResult.LOCK_OBTAIN_FAILED;
if (core.getCoreContainer().isShutDown()) {
log.warn("I was asked to replicate but CoreContainer is shutting down");
Expand Down Expand Up @@ -1196,7 +1196,7 @@ private void setupPolling(String intervalStr) {
try {
log.debug("Polling for index modifications");
markScheduledExecutionStart();
IndexFetchResult fetchResult = doFetch(null, false);
IndexFetchResult fetchResult = doFetch(SolrParams.of(), false);
if (pollListener != null) pollListener.onComplete(core, fetchResult);
} catch (Exception e) {
log.error("Exception in fetching index", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
Expand Down Expand Up @@ -53,10 +52,6 @@ public static void addExperimentalFormatWarning(SolrQueryResponse rsp) {
public static boolean handleCommit(
SolrQueryRequest req, UpdateRequestProcessor processor, SolrParams params, boolean force)
throws IOException {
if (params == null) {
params = new MapSolrParams(new HashMap<>());
}

boolean optimize = params.getBool(UpdateParams.OPTIMIZE, false);
boolean commit = params.getBool(UpdateParams.COMMIT, false);
boolean softCommit = params.getBool(UpdateParams.SOFT_COMMIT, false);
Expand Down Expand Up @@ -97,8 +92,6 @@ public static void validateCommitParams(SolrParams params) {

/** Modify UpdateCommand based on request parameters */
public static void updateCommit(CommitUpdateCommand cmd, SolrParams params) {
if (params == null) return;

cmd.openSearcher = params.getBool(UpdateParams.OPEN_SEARCHER, cmd.openSearcher);
cmd.waitSearcher = params.getBool(UpdateParams.WAIT_SEARCHER, cmd.waitSearcher);
cmd.softCommit = params.getBool(UpdateParams.SOFT_COMMIT, cmd.softCommit);
Expand All @@ -114,13 +107,7 @@ public static void updateCommit(CommitUpdateCommand cmd, SolrParams params) {
public static boolean handleRollback(
SolrQueryRequest req, UpdateRequestProcessor processor, SolrParams params, boolean force)
throws IOException {
if (params == null) {
params = new MapSolrParams(new HashMap<>());
}

boolean rollback = params.getBool(UpdateParams.ROLLBACK, false);

if (rollback || force) {
if (force || params.getBool(UpdateParams.ROLLBACK, false)) {
RollbackUpdateCommand cmd = new RollbackUpdateCommand(req);
processor.processRollback(cmd);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ public static void squashIntoNamedListWithoutHeader(
}

public static String getMediaTypeFromWtParam(SolrParams params, String defaultMediaType) {
if (params == null) {
return defaultMediaType;
}
final String wtParam = params.get(WT);
if (StrUtils.isBlank(wtParam)) return defaultMediaType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ public SampleDocuments parseDocsFromStream(
return SampleDocuments.NONE;
}

if (params == null) {
params = new ModifiableSolrParams();
}

Long streamSize = stream.getSize();
if (streamSize != null && streamSize > MAX_STREAM_SIZE) {
throw new SolrException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ public class IgnoreCommitOptimizeUpdateProcessorFactory extends UpdateRequestPro

@Override
public void init(final NamedList<?> args) {
SolrParams params = (args != null) ? args.toSolrParams() : null;
if (params == null) {
if (args == null) {
errorCode = ErrorCode.FORBIDDEN; // default is 403 error
responseMsg = DEFAULT_RESPONSE_MSG;
ignoreOptimizeOnly = false;
return;
}
SolrParams params = args.toSolrParams();

ignoreOptimizeOnly = params.getBool("ignoreOptimizeOnly", false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -241,8 +240,6 @@ protected void indexDoc(Object... fields) throws IOException, SolrServerExceptio

UpdateRequest ureq = new UpdateRequest();
ureq.add(doc);
ModifiableSolrParams params = new ModifiableSolrParams();
ureq.setParams(params);
ureq.process(cloudClient);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.metrics.SolrMetricManager;
import org.junit.Test;
Expand Down Expand Up @@ -369,8 +368,6 @@ protected void indexDoc(Object... fields) throws IOException, SolrServerExceptio

UpdateRequest ureq = new UpdateRequest();
ureq.add(doc);
ModifiableSolrParams params = new ModifiableSolrParams();
ureq.setParams(params);
ureq.process(cloudClient);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,11 +625,8 @@ protected long addDocAndGetVersion(Object... fields) throws Exception {
SolrInputDocument doc = new SolrInputDocument();
addFields(doc, fields);

ModifiableSolrParams params = new ModifiableSolrParams();
params.add("versions", "true");

UpdateRequest ureq = new UpdateRequest();
ureq.setParams(params);
ureq.getParams().set("versions", true);
ureq.add(doc);
UpdateResponse resp;

Expand All @@ -647,10 +644,10 @@ protected long addDocAndGetVersion(Object... fields) throws Exception {

protected long deleteDocAndGetVersion(
String id, ModifiableSolrParams params, boolean deleteByQuery) throws Exception {
params.add("versions", "true");

UpdateRequest ureq = new UpdateRequest();
ureq.setParams(params);
ureq.getParams().add(params);
ureq.getParams().set("versions", true);

if (deleteByQuery) {
ureq.deleteByQuery("id:" + id);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void setup() throws IOException {

@Test
public void testJson() throws Exception {
loadTestDocs(null, new File(exampleDir, "films/films.json"), 500, 500);
loadTestDocs(SolrParams.of(), new File(exampleDir, "films/films.json"), 500, 500);
}

@Test
Expand All @@ -78,7 +78,7 @@ public void testCsv() throws Exception {

@Test
public void testSolrXml() throws Exception {
loadTestDocs(null, new File(exampleDir, "films/films.xml"), 1000, 1000);
loadTestDocs(SolrParams.of(), new File(exampleDir, "films/films.xml"), 1000, 1000);
}

protected List<SolrInputDocument> loadTestDocs(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ public void read(SolrQueryRequest req, SolrQueryResponse rsp) {

private static SolrQueryResponse v2ApiInvoke(
ApiBag bag, String uri, String method, SolrParams params, InputStream payload) {
if (params == null) params = new ModifiableSolrParams();
SolrQueryResponse rsp = new SolrQueryResponse();
HashMap<String, String> templateVals = new HashMap<>();
Api[] currentApi = new Api[1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,15 @@ public void after() throws Exception {

public void testSimple() throws Exception {

ModifiableSolrParams params =
new ModifiableSolrParams()
.add("processor", "template")
.add("template.field", "id:{firstName}_{lastName}")
.add("template.field", "another:{lastName}_{firstName}")
.add("template.field", "missing:{lastName}_{unKnown}");
AddUpdateCommand cmd = new AddUpdateCommand(new LocalSolrQueryRequest(null, params));
var cmd =
new AddUpdateCommand(
new LocalSolrQueryRequest(
null,
new ModifiableSolrParams()
.add("processor", "template")
.add("template.field", "id:{firstName}_{lastName}")
.add("template.field", "another:{lastName}_{firstName}")
.add("template.field", "missing:{lastName}_{unKnown}")));

cmd.solrDoc = new SolrInputDocument();
cmd.solrDoc.addField("firstName", "Tom");
Expand All @@ -69,14 +71,11 @@ public void testSimple() throws Exception {
SolrInputDocument solrDoc = new SolrInputDocument();
solrDoc.addField("id", "1");

params =
new ModifiableSolrParams()
.add("processor", "template")
.add("commit", "true")
.add("template.field", "x_s:key_{id}");
params.add("commit", "true");
UpdateRequest add = new UpdateRequest().add(solrDoc);
add.setParams(params);
add.getParams()
.add("processor", "template")
.add("template.field", "x_s:key_{id}")
.add("commit", "true");
NamedList<Object> result =
cluster
.getSolrClient()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void testInvalidAdds() throws IOException {
assertAddsSucceedWithErrors(
"tolerant-chain-max-errors-10",
Arrays.asList(new SolrInputDocument[] {invalidDoc1}),
null,
SolrParams.of(),
"(unknown)");

// a valid doc
Expand All @@ -149,7 +149,7 @@ public void testInvalidAdds() throws IOException {
() ->
add(
"not-tolerant",
null,
SolrParams.of(),
Arrays.asList(new SolrInputDocument[] {invalidDoc1, validDoc1})));
assertTrue(e.getMessage().contains("Document is missing mandatory uniqueKey field"));

Expand All @@ -159,7 +159,7 @@ public void testInvalidAdds() throws IOException {
assertAddsSucceedWithErrors(
"tolerant-chain-max-errors-10",
Arrays.asList(new SolrInputDocument[] {invalidDoc1, validDoc1}),
null,
SolrParams.of(),
"(unknown)");
assertU(commit());

Expand All @@ -176,7 +176,7 @@ public void testInvalidAdds() throws IOException {
() ->
add(
"not-tolerant",
null,
SolrParams.of(),
Arrays.asList(new SolrInputDocument[] {invalidDoc2, validDoc2})));
assertTrue(e.getMessage().contains("Error adding field"));

Expand All @@ -186,7 +186,7 @@ public void testInvalidAdds() throws IOException {
assertAddsSucceedWithErrors(
"tolerant-chain-max-errors-10",
Arrays.asList(new SolrInputDocument[] {invalidDoc2, validDoc2}),
null,
SolrParams.of(),
"2");
assertU(commit());

Expand All @@ -201,7 +201,7 @@ public void testInvalidAdds() throws IOException {
public void testMaxErrorsDefault() throws IOException {
// by default the TolerantUpdateProcessor accepts all errors, so this batch should succeed with
// 10 errors.
assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, null, badIds);
assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, SolrParams.of(), badIds);
assertU(commit());
assertQ(req("q", "*:*"), "//result[@numFound='10']");
}
Expand Down Expand Up @@ -240,7 +240,7 @@ public void testMaxErrorsThrowsException() {
public void testMaxErrorsInfinite() throws IOException {
ModifiableSolrParams requestParams = new ModifiableSolrParams();
requestParams.add("maxErrors", "-1");
assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, null, badIds);
assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, SolrParams.of(), badIds);

assertU(commit());
assertQ(req("q", "*:*"), "//result[@numFound='10']");
Expand Down Expand Up @@ -421,7 +421,7 @@ private void assertAddsSucceedWithErrors(

protected SolrQueryResponse add(final String chain, final SolrInputDocument doc)
throws IOException {
return add(chain, null, Arrays.asList(new SolrInputDocument[] {doc}));
return add(chain, SolrParams.of(), Arrays.asList(new SolrInputDocument[] {doc}));
}

protected SolrQueryResponse add(
Expand All @@ -435,10 +435,6 @@ protected SolrQueryResponse add(
SolrQueryResponse rsp = new SolrQueryResponse();
rsp.add("responseHeader", new SimpleOrderedMap<>());

if (requestParams == null) {
requestParams = new ModifiableSolrParams();
}

SolrQueryRequest req = new LocalSolrQueryRequest(core, requestParams);
UpdateRequestProcessor processor = null;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ void preventCircularMirroring(MirroredSolrRequest<?> mirroredSolrRequest) {
}
} else {
SolrParams params = mirroredSolrRequest.getSolrRequest().getParams();
String shouldMirror = (params == null ? null : params.get(CrossDcConstants.SHOULD_MIRROR));
assert params != null;
String shouldMirror = params.get(CrossDcConstants.SHOULD_MIRROR);
if (shouldMirror == null) {
if (params instanceof ModifiableSolrParams) {
log.warn("{} param is missing - setting to false", CrossDcConstants.SHOULD_MIRROR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.SolrNamedThreadFactory;
import org.apache.solr.common.util.Utils;
Expand Down Expand Up @@ -279,7 +278,7 @@ public void testHandleValidMirroredSolrRequest() {
doc.addField("id", "1");
UpdateRequest validRequest = new UpdateRequest();
validRequest.add(doc);
validRequest.setParams(new ModifiableSolrParams().add("commit", "true"));
validRequest.getParams().set("commit", true);
// Create a valid MirroredSolrRequest
ConsumerRecord<String, MirroredSolrRequest<?>> record =
new ConsumerRecord<>("test-topic", 0, 0, "key", new MirroredSolrRequest<>(validRequest));
Expand Down Expand Up @@ -459,7 +458,7 @@ protected KafkaMirroringSink createKafkaMirroringSink(KafkaCrossDcConf conf) {

UpdateRequest invalidRequest = new UpdateRequest();
// no updates on request
invalidRequest.setParams(new ModifiableSolrParams().add("invalid_param", "invalid_value"));
invalidRequest.getParams().add("invalid_param", "invalid_value");

ConsumerRecord<String, MirroredSolrRequest<?>> record =
new ConsumerRecord<>("test-topic", 0, 0, "key", new MirroredSolrRequest<>(invalidRequest));
Expand Down
Loading

0 comments on commit bfdec90

Please sign in to comment.