Re: [PR] Fix DockMakerTest maxCardinality [solr]
dsmiley commented on code in PR #3171:
URL: https://github.com/apache/solr/pull/3171#discussion_r1948227396
##
solr/benchmark/src/test/org/apache/solr/bench/DockMakerTest.java:
##
@@ -200,25 +201,4 @@ public void testWordListZipfian() {
assertNotNull(field.getValue().toString());
}
-
- @Test
- public void testGenDoc() {
Review Comment:
removed because it had no assertions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17649: Fix Json faceting on multivalue number types [solr]
mkhludnev commented on code in PR #3158:
URL: https://github.com/apache/solr/pull/3158#discussion_r1948227521
##
solr/core/src/java/org/apache/solr/search/facet/FacetField.java:
##
@@ -139,11 +139,11 @@ public FacetProcessor
createFacetProcessor(FacetContext fcontext) {
// FieldType.getDocValuesType()
if (!multiToken) {
- if (mincount > 0 && prefix == null && (ntype != null || method ==
FacetMethod.DVHASH)) {
+ if (mincount > 0 && prefix == null && (isNumber || method ==
FacetMethod.DVHASH)) {
// TODO can we auto-pick for strings when term cardinality is much
greater than DocSet
// cardinality? or if we don't know cardinality but DocSet size is
very small
return new FacetFieldProcessorByHashDV(fcontext, this, sf);
- } else if (ntype == null) {
+ } else if (isNumber == false) {
Review Comment:
!isNumber
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17649: Fix Json faceting on multivalue number types [solr]
dsmiley commented on code in PR #3158:
URL: https://github.com/apache/solr/pull/3158#discussion_r1948246532
##
solr/core/src/java/org/apache/solr/search/facet/FacetField.java:
##
@@ -139,11 +139,11 @@ public FacetProcessor
createFacetProcessor(FacetContext fcontext) {
// FieldType.getDocValuesType()
if (!multiToken) {
- if (mincount > 0 && prefix == null && (ntype != null || method ==
FacetMethod.DVHASH)) {
+ if (mincount > 0 && prefix == null && (isNumber || method ==
FacetMethod.DVHASH)) {
// TODO can we auto-pick for strings when term cardinality is much
greater than DocSet
// cardinality? or if we don't know cardinality but DocSet size is
very small
return new FacetFieldProcessorByHashDV(fcontext, this, sf);
- } else if (ntype == null) {
+ } else if (isNumber == false) {
Review Comment:
Maybe you are not familiar that Lucene & Solr has recognized this style as
acceptable due to its clarity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17662 SPI: TransformerFactory/DocTransformer [solr]
dsmiley commented on code in PR #3172:
URL: https://github.com/apache/solr/pull/3172#discussion_r1948343274
##
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##
@@ -1129,7 +1129,7 @@ protected SolrCore(
initWriters();
qParserPlugins.init(QParserPlugin.standardPlugins, this);
valueSourceParsers.init(ValueSourceParser.standardValueSourceParsers,
this);
- transformerFactories.init(TransformerFactory.defaultFactories, this);
+
transformerFactories.init(TransformerFactory.builtIns(coreContainer.getConfig()),
this);
Review Comment:
I like the name "builtIns" best so I chose it. "implicits" would be my
second choice. I like a general name, thus not including "factories" as some
places where we would use this pattern are not even "factories". "default"
isn't terrible but less good.
##
solr/core/src/java/org/apache/solr/response/transform/TransformerFactory.java:
##
Review Comment:
There are some other TransformerFactory impls not listed here; 2 for
QueryElevationComponent which weirdly self-register when QEC loads, and the
other is LTR. I could expand the scope of this PR but wanted to start simpler
to get the conversation going.
##
solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java:
##
@@ -18,6 +18,7 @@
import static org.apache.solr.schema.IndexSchema.NEST_PATH_FIELD_NAME;
+import jakarta.inject.Named;
Review Comment:
To solve the problem of identifying the name for plugins, I found this
annotation, which seems quite suitable. I read further about it when looking
at HK2 (a dependency of ours) lightweight dependency-injection framework that
uses the annotations in that JAR/package. This PR doesn't use HK2 BTW.
##
solr/core/src/java/org/apache/solr/response/transform/TransformerFactory.java:
##
@@ -105,18 +107,17 @@ default boolean mayModifyValue() {
}
}
- public static final Map defaultFactories = new
HashMap<>(9, 1.0f);
+ // loaded once, based on the node
+ private static Map builtIns;
- static {
-defaultFactories.put("explain", new ExplainAugmenterFactory());
-defaultFactories.put("value", new ValueAugmenterFactory());
-defaultFactories.put("docid", new DocIdAugmenterFactory());
-defaultFactories.put("shard", new ShardAugmenterFactory());
-defaultFactories.put("child", new ChildDocTransformerFactory());
-defaultFactories.put("subquery", new SubQueryAugmenterFactory());
-defaultFactories.put("json", new RawValueTransformerFactory("json"));
-defaultFactories.put("xml", new RawValueTransformerFactory("xml"));
-defaultFactories.put("geo", new GeoTransformerFactory());
-defaultFactories.put("core", new CoreAugmenterFactory());
+ /** (for internal use) */
+ public static synchronized Map
builtIns(NodeConfig nodeConfig) {
Review Comment:
Since the lazy loaded list is initialized based on NodeConfig (basically
CoreContainer), admittedly there's a test stability risk since NodeConfig
ClassLoader classpath of the first test that runs will "win". For the built-in
plugins, let's just not do anything that would disturb that list. If we wanted
to test that someone could add a plugin in a JAR, that would be a high level
integration test using Docker or BATS. Not unit tests.
##
solr/core/src/java/org/apache/solr/response/transform/TransformerFactory.java:
##
@@ -105,18 +107,17 @@ default boolean mayModifyValue() {
}
}
- public static final Map defaultFactories = new
HashMap<>(9, 1.0f);
+ // loaded once, based on the node
+ private static Map builtIns;
Review Comment:
no static initializer anymore; no class loading deadlock risk. Right
@uschindler ?
##
solr/core/src/java/org/apache/solr/response/transform/CoreAugmenterFactory.java:
##
@@ -17,9 +17,11 @@
package org.apache.solr.response.transform;
+import jakarta.inject.Named;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.request.SolrQueryRequest;
+@Named("core")
public class CoreAugmenterFactory extends TransformerFactory {
Review Comment:
`` Why the hell didn't we name TransformerFactory as its rightful
name, DocTransformerFactory, and name it's implementations as such as well? I
don't even need to know who/why; it's just wrong. We have sinned and need to
make amends someday.``
##
solr/core/src/java/org/apache/solr/response/transform/TransformerFactory.java:
##
@@ -105,18 +107,17 @@ default boolean mayModifyValue() {
}
}
- public static final Map defaultFactories = new
HashMap<>(9, 1.0f);
+ // loaded once, based on the node
+ private static Map builtIns;
- static {
-defaultFactories.put("explain", new ExplainAugmenterFactory());
-defaultFactories.put("value", new ValueAugmenterFactory());
-defaultFactories.put("docid", new DocIdAugmenterFactory());
-defaultFactories.put("
[PR] Fix DockMakerTest maxCardinality [solr]
dsmiley opened a new pull request, #3171: URL: https://github.com/apache/solr/pull/3171 See this [thread](https://github.com/apache/solr/pull/254#discussion_r1917413526) > I see a repeating failure running DockMakerTest.testBasicCardinalityAlpha with seed E0BBCB76FA600308 in which every field value is the letter 'k'. This DSL framework is extremely different from any other code most people encounter so it took some time to narrow down the problem to where I'm commenting. This code will choose consecutive seeds for subsequent processing. (Consecutiveness is not a problem I think). For a cardinality of 2 and based on the random test seed, this long seed will be either 635258717 or 635258718. But the next stage will always produce 'k' for either of those seeds; the next stage produces a random string of length from 1 to 6. I think the fundamental fix is in the test; to recognize this is a *max*Cardinality, and weaken its assertions accordingly. I added some comments in StringsDSL. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16966: Add a first-class cache for OrdinalMaps [solr]
github-actions[bot] closed pull request #1899: SOLR-16966: Add a first-class cache for OrdinalMaps URL: https://github.com/apache/solr/pull/1899 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17662 SPI: TransformerFactory/DocTransformer [solr]
dsmiley commented on code in PR #3172: URL: https://github.com/apache/solr/pull/3172#discussion_r1948360244 ## solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java: ## @@ -18,6 +18,7 @@ import static org.apache.solr.schema.IndexSchema.NEST_PATH_FIELD_NAME; +import jakarta.inject.Named; Review Comment: Could also have added a getName to instances but it's more verbose and forces instantiation of the class. They are instantiated here anyway but nonetheless I like the idea of a future optimization that doesn't instantiate until first use. Could also decide a public static final String field `NAME` is the name. Anyway, FWIW I prefer the annotation; feels more modern and is simpler. Wouldn't be okay in SolrJ (e.g. for DocRouter) as we don't want to add dependencies there but that's fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17662 SPI: TransformerFactory/DocTransformer [solr]
dsmiley commented on PR #3172: URL: https://github.com/apache/solr/pull/3172#issuecomment-2646875261 Adding another check, that the implementations do *not* implement `SolrCoreAware`, would also be good, as they are the builtIn ones instantiated once. I imagine a non-issue for existing ones but I could imagine a user trying this feature out and it not working for that reason. Of course eventually we want that to work but it's out of scope of the built-in registry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16966: Add a first-class cache for OrdinalMaps [solr]
github-actions[bot] commented on PR #1899: URL: https://github.com/apache/solr/pull/1899#issuecomment-2646664483 This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16162: FilterQuery should implement DocSetProducer [solr]
github-actions[bot] commented on PR #814: URL: https://github.com/apache/solr/pull/814#issuecomment-2646664536 This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16707: Avoid "Recursive update" ISE in non-async cache `computeIfAbsent()` [solr]
github-actions[bot] closed pull request #1481: SOLR-16707: Avoid "Recursive update" ISE in non-async cache `computeIfAbsent()` URL: https://github.com/apache/solr/pull/1481 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16707: Avoid "Recursive update" ISE in non-async cache `computeIfAbsent()` [solr]
github-actions[bot] commented on PR #1481: URL: https://github.com/apache/solr/pull/1481#issuecomment-2646664503 This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16162: FilterQuery should implement DocSetProducer [solr]
github-actions[bot] closed pull request #814: SOLR-16162: FilterQuery should implement DocSetProducer URL: https://github.com/apache/solr/pull/814 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Created] (SOLR-17662) Java SPI/ServiceLoader for plugin discovery & instantiation
David Smiley created SOLR-17662: --- Summary: Java SPI/ServiceLoader for plugin discovery & instantiation Key: SOLR-17662 URL: https://issues.apache.org/jira/browse/SOLR-17662 Project: Solr Issue Type: Improvement Reporter: David Smiley Java 6 introduced the [Service Provider Interfaces|https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html] mechanism, implemented with the ServiceLoader class. As applied to Solr, it allows a registry of plugin instances of a certain type to be discovered from the classpath. This obsoletes static hard-coded mappings (see Solr's TransformerFactory for one), and it allows easy addition of new plugin instances by others simply by supplying a JAR with metadata, without requiring modifications to solrconfig.xml or other config files (assuming zero-config). A challenge to solve is how to get the basic name of a plugins, e.g. "subquery" instead of the full classname: org.apache.solr.response.transform.SubQueryAugmenterFactory A short term goal could just be Solr's static registries of builtin plugins using Solr's classpath. Slightly better is using the node level ClassLoader, thereby using the configurable lib dir. More comprehensive would eventually allow configSets and the package system somehow. Eventually allow referencing these plugin names in configuration (class="subquery" or name="subquery") instead of "solr.SubQueryAugmenterFactory" for explicitly configuring. The latter should be deprecated despite its use in Solr since forever. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[PR] SOLR-17662 SPI: TransformerFactory/DocTransformer [solr]
dsmiley opened a new pull request, #3172: URL: https://github.com/apache/solr/pull/3172 https://issues.apache.org/jira/browse/SOLR-17662 Despite me wanting this for a long time, what motivated me today specifically was looking at @malliaridis trying to solve risks of static class initialization for some of these factories in #2881. The use of SPIs should make that concern go away for some things like static plugin registries. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-17662) Java SPI/ServiceLoader for plugin discovery & instantiation
[ https://issues.apache.org/jira/browse/SOLR-17662?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] ASF GitHub Bot updated SOLR-17662: -- Labels: pull-request-available (was: ) > Java SPI/ServiceLoader for plugin discovery & instantiation > --- > > Key: SOLR-17662 > URL: https://issues.apache.org/jira/browse/SOLR-17662 > Project: Solr > Issue Type: Improvement >Reporter: David Smiley >Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > Java 6 introduced the [Service Provider > Interfaces|https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html] > mechanism, implemented with the ServiceLoader class. As applied to Solr, it > allows a registry of plugin instances of a certain type to be discovered from > the classpath. This obsoletes static hard-coded mappings (see Solr's > TransformerFactory for one), and it allows easy addition of new plugin > instances by others simply by supplying a JAR with metadata, without > requiring modifications to solrconfig.xml or other config files (assuming > zero-config). > A challenge to solve is how to get the basic name of a plugins, e.g. > "subquery" instead of the full classname: > org.apache.solr.response.transform.SubQueryAugmenterFactory > A short term goal could just be Solr's static registries of builtin plugins > using Solr's classpath. Slightly better is using the node level ClassLoader, > thereby using the configurable lib dir. More comprehensive would eventually > allow configSets and the package system somehow. Eventually allow > referencing these plugin names in configuration (class="subquery" or > name="subquery") instead of "solr.SubQueryAugmenterFactory" for explicitly > configuring. The latter should be deprecated despite its use in Solr since > forever. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17628: Add query quantiles metrics to prometheus endpoint [solr]
dsmiley commented on code in PR #3164:
URL: https://github.com/apache/solr/pull/3164#discussion_r1948435790
##
solr/core/src/java/org/apache/solr/metrics/prometheus/SolrPrometheusFormatter.java:
##
@@ -206,6 +251,22 @@ public void collectGaugeDatapoint(
metricGauges.get(metricName).add(dataPoint);
}
+ /**
+ * Collects {@link
io.prometheus.metrics.model.snapshots.SummarySnapshot.SummaryDataPointSnapshot}
+ * and appends to existing metric or create new metric if name does not exist
+ *
+ * @param metricName Name of metric
+ * @param dataPoint Gauge datapoint to be collected
+ */
+ public void collectSummaryDatapoint(
+ String metricName, SummarySnapshot.SummaryDataPointSnapshot dataPoint) {
+if (!metricSummaries.containsKey(metricName)) {
+ metricSummaries.put(metricName, new ArrayList<>());
+}
+
+metricSummaries.get(metricName).add(dataPoint);
Review Comment:
```suggestion
metricSummaries.computeIfAbsent(metricName, k -> new
ArrayList<>()).add(dataPoint);
```
##
solr/core/src/java/org/apache/solr/metrics/prometheus/SolrPrometheusFormatter.java:
##
@@ -176,6 +202,25 @@ public GaugeSnapshot.GaugeDataPointSnapshot
createGaugeDatapoint(double value, L
return
GaugeSnapshot.GaugeDataPointSnapshot.builder().value(value).labels(labels).build();
}
+ /**
+ * Create a {@link
io.prometheus.metrics.model.snapshots.SummarySnapshot.SummaryDataPointSnapshot}
+ * with labels
+ *
+ * @param count number of observations
+ * @param sum sum of all values
+ * @param quantiles quantile information
+ * @param labels set of name/values labels
+ */
+ public SummarySnapshot.SummaryDataPointSnapshot
createSummaryDataPointSnapshot(
+ long count, double sum, Quantiles quantiles, Labels labels) {
+return SummarySnapshot.SummaryDataPointSnapshot.builder()
+.count(count)
+.sum(sum)
+.labels(labels)
+.quantiles(quantiles)
+.build();
+ }
Review Comment:
I don't see why this is its own method, as you now feel the need to have
quite a number of lines here (including documentation). Just inline it.
##
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHandlerMetric.java:
##
@@ -28,7 +28,7 @@ public class SolrCoreHandlerMetric extends SolrCoreMetric {
public static final String CORE_REQUESTS_TOTAL =
"solr_metrics_core_requests";
public static final String CORE_REQUESTS_UPDATE_HANDLER =
"solr_metrics_core_update_handler";
public static final String CORE_REQUESTS_TOTAL_TIME =
"solr_metrics_core_requests_time";
- public static final String CORE_REQUEST_TIMES =
"solr_metrics_core_average_request_time";
+ public static final String CORE_REQUEST_TIMES =
"solr_metrics_core_request_time";
Review Comment:
I love that feedback in general but not sure if there is a consistency issue
(like do this not just here but basically everywhere)
##
solr/core/src/java/org/apache/solr/metrics/prometheus/SolrPrometheusFormatter.java:
##
@@ -220,6 +281,11 @@ public MetricSnapshots collect() {
snapshots.add(
new GaugeSnapshot(new MetricMetadata(metricName),
metricGauges.get(metricName)));
}
+for (String metricName : metricSummaries.keySet()) {
Review Comment:
Always iterate the entrySet or call forEach so you don't have to lookup the
value of each one separately
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115:
URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1948445957
##
encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java:
##
@@ -412,58 +409,46 @@ private void distributeRequest(SolrQueryRequest req,
SolrQueryResponse rsp, Stri
rsp.addToLog(ENCRYPTION_STATE, collectionState.value);
}
if (log.isInfoEnabled()) {
-long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() -
startTimeNs);
-log.info("Responding encryption distributed state={} success={} for
keyId={} collection={} timeMs={}",
-(collectionState == null ? null : collectionState.value), success,
keyId, collectionName, timeMs);
+log.info("Responding encryption distributed state={} success={} for
keyId={} timeMs={}",
+(collectionState == null ? null : collectionState.value), success,
keyId, elapsedTimeMs(startTimeNs));
}
}
}
- private SolrClientHolder getHttpSolrClient(SolrQueryRequest req) {
-CoreContainer coreContainer = req.getCore().getCoreContainer();
-CloudSolrClient cloudSolrClient =
coreContainer.getSolrClientCache().getCloudSolrClient(coreContainer.getZkController().getZkClient().getZkServerAddress());
-if (cloudSolrClient instanceof CloudHttp2SolrClient) {
- return new SolrClientHolder(((CloudHttp2SolrClient)
cloudSolrClient).getHttpClient(), false);
-}
-return new SolrClientHolder(new Http2SolrClient.Builder().build(), true);
- }
-
protected ModifiableSolrParams
createDistributedRequestParams(SolrQueryRequest req, SolrQueryResponse rsp,
String keyId) {
-ModifiableSolrParams params = new ModifiableSolrParams();
-params.set(PARAM_KEY_ID, keyId == null ? NO_KEY_ID : keyId);
-return params;
- }
-
- boolean isTimeout(long maxTimeNs) {
-return System.nanoTime() > maxTimeNs;
+return new ModifiableSolrParams().set(PARAM_KEY_ID, keyId == null ?
NO_KEY_ID : keyId);
}
- private State sendEncryptionRequestWithRetry(Replica replica,
ModifiableSolrParams params, Http2SolrClient httpSolrClient, String keyId,
String collection) {
+ private State sendEncryptionRequestWithRetry(
+ Replica replica,
+ String requestPath,
+ ModifiableSolrParams params,
+ Http2SolrClient httpSolrClient,
+ String keyId) {
for (int numAttempts = 0; numAttempts < DISTRIBUTION_MAX_ATTEMPTS;
numAttempts++) {
try {
-NamedList response = sendEncryptionRequest(replica, params,
httpSolrClient);
-Object responseStatus = response.get(STATUS);
-Object responseState = response.get(ENCRYPTION_STATE);
-log.info("Encryption state {} status {} for replica {} keyId {}
collection={}", responseStatus, responseState, replica.getName(), keyId,
collection);
+SimpleSolrResponse response = sendEncryptionRequest(replica,
requestPath, params, httpSolrClient);
+Object responseStatus = response.getResponse().get(STATUS);
+Object responseState = response.getResponse().get(ENCRYPTION_STATE);
+log.info("Encryption state {} status {} for replica {} keyId {} in {}
ms", responseStatus, responseState, replica.getName(), keyId,
response.getElapsedTime());
if (responseState != null) {
return State.fromValue(responseState.toString());
}
} catch (SolrServerException | IOException e) {
log.error("Error occurred while sending encryption request", e);
}
}
-throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed
encryption request to replica " + replica.getName() + " for keyId " + keyId + "
collection " + collection);
+throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed
encryption request to replica " + replica.getName() + " for keyId " + keyId);
}
- private NamedList sendEncryptionRequest(Replica replica,
ModifiableSolrParams params, Http2SolrClient httpSolrClient)
+ private SimpleSolrResponse sendEncryptionRequest(
+ Replica replica,
+ String requestPath,
+ ModifiableSolrParams params,
+ Http2SolrClient httpSolrClient)
throws SolrServerException, IOException {
-GenericSolrRequest request = new
GenericSolrRequest(SolrRequest.METHOD.POST, getPath(), params);
-request.setBasePath(replica.getCoreUrl());
-return httpSolrClient.request(request);
- }
-
- protected String getPath() {
-return "/admin/encrypt";
+GenericSolrRequest distributedRequest = new
GenericSolrRequest(SolrRequest.METHOD.POST, requestPath, params);
+return httpSolrClient.requestWithBaseUrl(replica.getCoreUrl(),
replica.getCollection(), distributedRequest);
Review Comment:
CC @gerlowskija I propose
`org.apache.solr.client.solrj.util.ClientUtils#buildRequestUrl` fail with
`IllegalArgumentException` if `collection` is passed in but isn't used.
--
This is an automated message from the Apache Git Service.
To respond to the messag
Re: [PR] SOLR-17649: Fix Json faceting on multivalue number types [solr]
mkhludnev commented on code in PR #3158:
URL: https://github.com/apache/solr/pull/3158#discussion_r1948229222
##
solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java:
##
@@ -44,27 +48,49 @@ public static SortedDocValues
getSortedDocValues(SolrIndexSearcher searcher, Str
public static SortedDocValues getSortedDocValues(
QueryContext context, SchemaField field, QParser qparser) throws
IOException {
-SortedDocValues si =
-
context.searcher().getSlowAtomicReader().getSortedDocValues(field.getName());
-// if (!field.hasDocValues() && (field.getType() instanceof StrField ||
field.getType()
-// instanceof TextField)) {
-// }
-
-return si == null ? DocValues.emptySorted() : si;
+var reader = context.searcher().getSlowAtomicReader();
+var dv = reader.getSortedDocValues(field.getName());
+checkDvType(dv, field, reader);
+return dv == null ? DocValues.emptySorted() : dv;
}
public static SortedSetDocValues getSortedSetDocValues(
QueryContext context, SchemaField field, QParser qparser) throws
IOException {
-SortedSetDocValues si =
-
context.searcher().getSlowAtomicReader().getSortedSetDocValues(field.getName());
-return si == null ? DocValues.emptySortedSet() : si;
+var reader = context.searcher().getSlowAtomicReader();
+var dv = reader.getSortedSetDocValues(field.getName());
+checkDvType(dv, field, reader);
+return dv == null ? DocValues.emptySortedSet() : dv;
}
public static NumericDocValues getNumericDocValues(
QueryContext context, SchemaField field, QParser qparser) throws
IOException {
-SolrIndexSearcher searcher = context.searcher();
-NumericDocValues si =
searcher.getSlowAtomicReader().getNumericDocValues(field.getName());
-return si == null ? DocValues.emptyNumeric() : si;
+var reader = context.searcher().getSlowAtomicReader();
+var dv = reader.getNumericDocValues(field.getName());
+checkDvType(dv, field, reader);
+return dv == null ? DocValues.emptyNumeric() : dv;
+ }
+
+ private static void checkDvType(Object dv, SchemaField field, LeafReader
reader) {
Review Comment:
It's not clear why to have this 20 lines. If we remove it, I think it just
fails at class cast somewhere later. Why ISE is better than ClassCastException
later? It's just not obvious.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17649: Fix Json faceting on multivalue number types [solr]
dsmiley commented on code in PR #3158:
URL: https://github.com/apache/solr/pull/3158#discussion_r1948247603
##
solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java:
##
@@ -44,27 +48,49 @@ public static SortedDocValues
getSortedDocValues(SolrIndexSearcher searcher, Str
public static SortedDocValues getSortedDocValues(
QueryContext context, SchemaField field, QParser qparser) throws
IOException {
-SortedDocValues si =
-
context.searcher().getSlowAtomicReader().getSortedDocValues(field.getName());
-// if (!field.hasDocValues() && (field.getType() instanceof StrField ||
field.getType()
-// instanceof TextField)) {
-// }
-
-return si == null ? DocValues.emptySorted() : si;
+var reader = context.searcher().getSlowAtomicReader();
+var dv = reader.getSortedDocValues(field.getName());
+checkDvType(dv, field, reader);
+return dv == null ? DocValues.emptySorted() : dv;
}
public static SortedSetDocValues getSortedSetDocValues(
QueryContext context, SchemaField field, QParser qparser) throws
IOException {
-SortedSetDocValues si =
-
context.searcher().getSlowAtomicReader().getSortedSetDocValues(field.getName());
-return si == null ? DocValues.emptySortedSet() : si;
+var reader = context.searcher().getSlowAtomicReader();
+var dv = reader.getSortedSetDocValues(field.getName());
+checkDvType(dv, field, reader);
+return dv == null ? DocValues.emptySortedSet() : dv;
}
public static NumericDocValues getNumericDocValues(
QueryContext context, SchemaField field, QParser qparser) throws
IOException {
-SolrIndexSearcher searcher = context.searcher();
-NumericDocValues si =
searcher.getSlowAtomicReader().getNumericDocValues(field.getName());
-return si == null ? DocValues.emptyNumeric() : si;
+var reader = context.searcher().getSlowAtomicReader();
+var dv = reader.getNumericDocValues(field.getName());
+checkDvType(dv, field, reader);
+return dv == null ? DocValues.emptyNumeric() : dv;
+ }
+
+ private static void checkDvType(Object dv, SchemaField field, LeafReader
reader) {
Review Comment:
There's wasn't a class cast; there was empty/no data which ultimately leads
to no facet results. That's *worse* than throwing an exception (of whatever
type; no strong opinion from me). This is my primary addition to the PR; don't
*just* fix the wrong algorithm choice, but ensure we throw an exception in
cases that would previously silently produce nothing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] chore(deps): update dependency com.diffplug.spotless to v6.25.0 [solr]
malliaridis commented on PR #3082: URL: https://github.com/apache/solr/pull/3082#issuecomment-2646197885 We should upgrade this plugin instead and backport it. See https://github.com/apache/solr/pull/3125 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16427: Enable error-prone ClassInitializationDeadlock rule [solr]
malliaridis commented on PR #2881:
URL: https://github.com/apache/solr/pull/2881#issuecomment-2646309035
Thanks for your feedback @uschindler 😄
> Please be still careful to not use the plural s classes during
initialization on the original class. This would reintroduce the problems.
I believe this is not the case, and I hope error-prone would identify such
cases.
> If you have accessor methods, you can use the holder pattern used in
Lucene.
I haven't used this pattern before. Would
[`TimeSources.get(type)`](https://github.com/apache/solr/blob/b34abd5a5016f125174cd036cd7b3273efe6345a/solr/solrj/src/java/org/apache/solr/common/util/TimeSources.java#L49)
(previously `TimeSource.get(type)`) be such a scenario where this pattern
could be used?
I guess in cases where the instances are public and directly accessed or
added to a list with `static { ... }` the holder pattern cannot be used?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] chore(deps): update dependency org.ow2.asm:asm to v9.7.1 [solr]
malliaridis commented on PR #3110: URL: https://github.com/apache/solr/pull/3110#issuecomment-2646208813 Since `versions.props` in `branch_9x` does not explicitly defines the asm version, this PR won't be backported. For future reference, the explicit definition of this dependency for version alignment may be removed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16427: Enable error-prone ClassInitializationDeadlock rule [solr]
uschindler commented on PR #2881: URL: https://github.com/apache/solr/pull/2881#issuecomment-2646291916 Please be still careful to not use the plural s classes during initialization on the original class. This would reintroduce the problems. I wasn't able to check all those patterns, but I hope the error probe checker would gave found those, too. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115:
URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1948115545
##
encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java:
##
@@ -412,58 +409,46 @@ private void distributeRequest(SolrQueryRequest req,
SolrQueryResponse rsp, Stri
rsp.addToLog(ENCRYPTION_STATE, collectionState.value);
}
if (log.isInfoEnabled()) {
-long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() -
startTimeNs);
-log.info("Responding encryption distributed state={} success={} for
keyId={} collection={} timeMs={}",
-(collectionState == null ? null : collectionState.value), success,
keyId, collectionName, timeMs);
+log.info("Responding encryption distributed state={} success={} for
keyId={} timeMs={}",
+(collectionState == null ? null : collectionState.value), success,
keyId, elapsedTimeMs(startTimeNs));
}
}
}
- private SolrClientHolder getHttpSolrClient(SolrQueryRequest req) {
-CoreContainer coreContainer = req.getCore().getCoreContainer();
-CloudSolrClient cloudSolrClient =
coreContainer.getSolrClientCache().getCloudSolrClient(coreContainer.getZkController().getZkClient().getZkServerAddress());
-if (cloudSolrClient instanceof CloudHttp2SolrClient) {
- return new SolrClientHolder(((CloudHttp2SolrClient)
cloudSolrClient).getHttpClient(), false);
-}
-return new SolrClientHolder(new Http2SolrClient.Builder().build(), true);
- }
-
protected ModifiableSolrParams
createDistributedRequestParams(SolrQueryRequest req, SolrQueryResponse rsp,
String keyId) {
-ModifiableSolrParams params = new ModifiableSolrParams();
-params.set(PARAM_KEY_ID, keyId == null ? NO_KEY_ID : keyId);
-return params;
- }
-
- boolean isTimeout(long maxTimeNs) {
-return System.nanoTime() > maxTimeNs;
+return new ModifiableSolrParams().set(PARAM_KEY_ID, keyId == null ?
NO_KEY_ID : keyId);
}
- private State sendEncryptionRequestWithRetry(Replica replica,
ModifiableSolrParams params, Http2SolrClient httpSolrClient, String keyId,
String collection) {
+ private State sendEncryptionRequestWithRetry(
+ Replica replica,
+ String requestPath,
+ ModifiableSolrParams params,
+ Http2SolrClient httpSolrClient,
+ String keyId) {
for (int numAttempts = 0; numAttempts < DISTRIBUTION_MAX_ATTEMPTS;
numAttempts++) {
try {
-NamedList response = sendEncryptionRequest(replica, params,
httpSolrClient);
-Object responseStatus = response.get(STATUS);
-Object responseState = response.get(ENCRYPTION_STATE);
-log.info("Encryption state {} status {} for replica {} keyId {}
collection={}", responseStatus, responseState, replica.getName(), keyId,
collection);
+SimpleSolrResponse response = sendEncryptionRequest(replica,
requestPath, params, httpSolrClient);
+Object responseStatus = response.getResponse().get(STATUS);
+Object responseState = response.getResponse().get(ENCRYPTION_STATE);
+log.info("Encryption state {} status {} for replica {} keyId {} in {}
ms", responseStatus, responseState, replica.getName(), keyId,
response.getElapsedTime());
if (responseState != null) {
return State.fromValue(responseState.toString());
}
} catch (SolrServerException | IOException e) {
log.error("Error occurred while sending encryption request", e);
}
}
-throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed
encryption request to replica " + replica.getName() + " for keyId " + keyId + "
collection " + collection);
+throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed
encryption request to replica " + replica.getName() + " for keyId " + keyId);
}
- private NamedList sendEncryptionRequest(Replica replica,
ModifiableSolrParams params, Http2SolrClient httpSolrClient)
+ private SimpleSolrResponse sendEncryptionRequest(
+ Replica replica,
+ String requestPath,
+ ModifiableSolrParams params,
+ Http2SolrClient httpSolrClient)
throws SolrServerException, IOException {
-GenericSolrRequest request = new
GenericSolrRequest(SolrRequest.METHOD.POST, getPath(), params);
-request.setBasePath(replica.getCoreUrl());
-return httpSolrClient.request(request);
- }
-
- protected String getPath() {
-return "/admin/encrypt";
+GenericSolrRequest distributedRequest = new
GenericSolrRequest(SolrRequest.METHOD.POST, requestPath, params);
+return httpSolrClient.requestWithBaseUrl(replica.getCoreUrl(),
replica.getCollection(), distributedRequest);
Review Comment:
Thanks for the info. It's not obvious from the
Http2SolrClient.requestWithBaseUrl doc that the collection name is concatenated.
When looking at the internals, it seems to me the concatenation happens in
ClientUtils.buildRequestUrl, and it checks if so
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925379#comment-17925379 ] Gus Heck commented on SOLR-17659: - Another facet of what I'm saying is that the login should be tiny and easy to audit relative to the overall UI. > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925377#comment-17925377 ] Gus Heck commented on SOLR-17659: - What I'm advocating is that unauthenticated users cannot load the browser based client (application, aka ui) and use it to look for ways we have accidentally exposed information or features that are not properly protected. The login page should be a separate app (installed at the root context so it can set cookies/Authorization/etc that the browser will happily send to /solr /api etc.). I'm happy to agree that this ticket only handles Basic Auth but we should do so in a structure suitable for all [use cases|https://solr.apache.org/guide/solr/latest/deployment-guide/authentication-and-authorization-plugins.html#enabling-an-authentication-plugin]. That means we need to identify the flows for all use cases and understand how this form will satisfy them even if we don't create the plumbing to actually do so. Otherwise we likely wind up with three login forms that have to be enabled/disabled depending on what's being done, or a spiderweb of code dispatching on auth type that makes it very difficult to add new auth types when they come along. An independent app that establishes auth (either with a form or by redirecting to an external provider) and then redirects to the /ui context (or provides that redirect url to the external provider) to load the browser based ui application hopefully is reusable in all cases (if not I'm of course open to other solutions). Friendly 503 pages etc should just use the jetty error page functionality. Also I think it's fine for this ticket to only cover the UI side of it, anything that needs to change in core Solr to allow for sanity here can be a linked ticket. As a side note I'd expect that if we build it this way it would be easy for the same login form to * redirect to the old UI * redirect to the new UI (config option) * redirect to a custom UI written by a third party and added in to solr * redirect to a custom UI written by a third party hosted elsewhere (perhaps requires some as yet undeveloped proxy magic in cases that rely on basic or schemes involving cookies?) > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115:
URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1948118413
##
encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java:
##
@@ -527,31 +512,33 @@ private void commitEncryptionStart(String keyId,
req.getCore().getUpdateHandler().commit(commitCmd);
}
- private void encryptAsync(SolrQueryRequest req, long startTimeNs) {
+ private void encryptAsync(SolrQueryRequest req) {
log.debug("Submitting async encryption");
executor.submit(() -> {
try {
EncryptionUpdateLog updateLog = getEncryptionUpdateLog(req.getCore());
if (updateLog != null) {
log.debug("Encrypting update log");
+ long startTimeNs = getTimeSource().getTimeNs();
boolean logEncryptionComplete = updateLog.encryptLogs();
- log.info("{} encrypted the update log in {}",
- logEncryptionComplete ? "Successfully" : "Partially",
elapsedTime(startTimeNs));
+ log.info("{} encrypted the update log in {} ms",
+ logEncryptionComplete ? "Successfully" : "Partially",
elapsedTimeMs(startTimeNs));
// If the logs encryption is not complete, it means some logs are
currently in use.
// The encryption will be automatically be retried after the next
commit which should
// release the old transaction log and make it ready for encryption.
}
log.debug("Triggering index encryption");
+long startTimeNs = getTimeSource().getTimeNs();
CommitUpdateCommand commitCmd = new CommitUpdateCommand(req, true);
// Trigger EncryptionMergePolicy.findForcedMerges() to re-encrypt
// each segment which is not encrypted with the latest active key id.
commitCmd.maxOptimizeSegments = Integer.MAX_VALUE;
req.getCore().getUpdateHandler().commit(commitCmd);
-log.info("Successfully triggered index encryption with commit in {}",
elapsedTime(startTimeNs));
+log.info("Successfully triggered index encryption with commit in {}
ms", elapsedTimeMs(startTimeNs));
Review Comment:
I want to keep TimeSource to be able to inject a mock time source in tests.
So I'll separate the computation of the elapsed time in ns, and the conversion
to ms in another method for clarity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925383#comment-17925383 ] Jan Høydahl commented on SOLR-17659: Ui is only yet another solr client. No data. No special access. Any client must be granted direct access to Solrs backend port 8983. Thus whether you use a UI or cURL or SolrJ, you will gain access to exactly the same APIs and data, depending on how auth/autz is configured in solr. > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] chore(deps): update dependency org.ow2.asm:asm to v9.7.1 [solr]
solrbot commented on PR #3110: URL: https://github.com/apache/solr/pull/3110#issuecomment-2646205282 ### Edited/Blocked Notification Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. ⚠️ **Warning**: custom changes will be lost. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] chore(deps): update dependency org.ow2.asm:asm to v9.7.1 [solr]
malliaridis merged PR #3110: URL: https://github.com/apache/solr/pull/3110 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925389#comment-17925389 ] Gus Heck commented on SOLR-17659: - Yes I believe I said just that. > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16427: Enable error-prone ClassInitializationDeadlock rule [solr]
uschindler commented on PR #2881:
URL: https://github.com/apache/solr/pull/2881#issuecomment-2646355888
> Thanks for your feedback @uschindler 😄
>
> > Please be still careful to not use the plural s classes during
initialization on the original class. This would reintroduce the problems.
>
> I believe this is not the case, and I hope error-prone would identify such
cases.
OK.
> > If you have accessor methods, you can use the holder pattern used in
Lucene.
>
> I haven't used this pattern before. Would
[`TimeSources.get(type)`](https://github.com/apache/solr/blob/b34abd5a5016f125174cd036cd7b3273efe6345a/solr/solrj/src/java/org/apache/solr/common/util/TimeSources.java#L49)
(previously `TimeSource.get(type)`) be such a scenario where this pattern
could be used?
The pattern can be seen here when codec initialization is done. Lucene has a
default Codec, which is a subclass of the Codec class. The Codec class has a
method `Codec#getDefault()` which returns the static default. The
initialization is delayed by a Holder pattern:
https://github.com/apache/lucene/blob/30d18dfbe1e67bed2d4550b2bc20b9b86ce9781c/lucene/core/src/java/org/apache/lucene/codecs/Codec.java#L40-L59
Here it is called:
https://github.com/apache/lucene/blob/30d18dfbe1e67bed2d4550b2bc20b9b86ce9781c/lucene/core/src/java/org/apache/lucene/codecs/Codec.java#L139-L147
Basically the trick is similar to your's but has more checks to really
prevent codec subclasses to get the default during initialization phase (that's
why the null checks are there).
> I guess in cases where the instances are public and directly accessed or
added to a list with `static { ... }` the holder pattern cannot be used?
No, that's the problem of most classes seen in this PR. It is impossible to
use that where the superclass has a static final field to one of the
subclasses. You need to break backwards compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Comment Edited] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925389#comment-17925389 ] Gus Heck edited comment on SOLR-17659 at 2/9/25 3:28 PM: - Yes I believe I said that. was (Author: gus_heck): Yes I believe I said just that. > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925394#comment-17925394 ] Gus Heck commented on SOLR-17659: - To be clearer, think of the browser based UI as a pre-made list of URLs and Features to try to exploit. I would prefer not to allow attackers to be able to download a guided tour of things to hack from my server. > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925396#comment-17925396 ] Gus Heck commented on SOLR-17659: - I also hope that the login stuff is kept simple enough that it rarely changes and attackers have difficulty identifying the version of Solr based on it (and thus don't know what exploits to focus on). > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, if > he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. > The credentials do not have to survive an application restart (desktop). > Storing credentials securely will be addressed in a separate issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-17659) Implement basic authentication in Admin UI
[ https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christos Malliaridis updated SOLR-17659: Description: In the new UI one of the key features that is not implemented yet is user authentication. In order to secure and securily access Solr, the user should be able to authenticate against a Solr instance with basic credentials. h2. Task Implement basic user authentication (with credentials) according to the [new designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. h2. Acceptance Criteria - The user can access a Solr instance that has user authentication enabled - The user can at least authenticate with credentials (basic auth) - The credentials form is displayed after the user has established a connection with a Solr instance, that is, after a Solr instance was found - The user can return to the start screen where the Solr URL was provided, if he decides to abort the authentication step - The user is no longer redirected to the dashboard or any other screen if user authentication is required - The credentials are used for any subsequent request h2. Additional Information The support for additional authentication options does not have to be addressed in this issue. If it proves to be straight-forward, feel free to implement additional auth options as well. Note that additional authentication options will be added later, and therefore, the implementation should be expandable. The credentials do not have to survive an application restart (desktop). Storing credentials securely will be addressed in a separate issue. was: In the new UI one of the key features that is not implemented yet is user authentication. In order to secure and securily access Solr, the user should be able to authenticate against a Solr instance with basic credentials. h2. Task Implement basic user authentication (with credentials) according to the [new designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. h2. Acceptance Criteria - The user can access a Solr instance that has user authentication enabled - The user can at least authenticate with credentials (basic auth) - The credentials form is displayed after the user has established a connection with a Solr instance, that is, after a Solr instance was found - The user can return to the start screen where the Solr URL was provided, if he decides to abort the authentication step - The user is no longer redirected to the dashboard or any other screen if user authentication is required - The credentials are used for any subsequent request h2. Additional Information The support for additional authentication options does not have to be addressed in this issue. If it proves to be straight-forward, feel free to implement additional auth options as well. The credentials do not have to survive an application restart (desktop). Storing credentials securely will be addressed in a separate issue. > Implement basic authentication in Admin UI > -- > > Key: SOLR-17659 > URL: https://issues.apache.org/jira/browse/SOLR-17659 > Project: Solr > Issue Type: New Feature > Components: Admin UI >Reporter: Christos Malliaridis >Priority: Major > Labels: new-ui, ui > > In the new UI one of the key features that is not implemented yet is user > authentication. In order to secure and securily access Solr, the user should > be able to authenticate against a Solr instance with basic credentials. > h2. Task > Implement basic user authentication (with credentials) according to the [new > designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1]. > h2. Acceptance Criteria > - The user can access a Solr instance that has user authentication enabled > - The user can at least authenticate with credentials (basic auth) > - The credentials form is displayed after the user has established a > connection with a Solr instance, that is, after a Solr instance was found > - The user can return to the start screen where the Solr URL was provided, > if he decides to abort the authentication step > - The user is no longer redirected to the dashboard or any other screen if > user authentication is required > - The credentials are used for any subsequent request > h2. Additional Information > The support for additional authentication options does not have to be > addressed in this issue. If it proves to be straight-forward, feel free to > implement additional auth options as well. Note that additional > authentication options will be added later, and therefore, the implementation > should be expand
Re: [PR] chore(deps): update dependency org.owasp.dependencycheck to v12 [solr]
malliaridis merged PR #3138: URL: https://github.com/apache/solr/pull/3138 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] chore(deps): update dependency org.owasp.dependencycheck to v9.2.0 [solr]
malliaridis closed pull request #3111: chore(deps): update dependency org.owasp.dependencycheck to v9.2.0 URL: https://github.com/apache/solr/pull/3111 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] chore(deps): update dependency org.owasp.dependencycheck to v9.2.0 [solr]
malliaridis commented on PR #3111: URL: https://github.com/apache/solr/pull/3111#issuecomment-2646436311 Closing this as we have upgraded to 12.0.2 (see #3138) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[PR] Backport chore(deps): update dependency org.owasp.dependencycheck to v12 (#3138) [solr]
malliaridis opened a new pull request, #3170: URL: https://github.com/apache/solr/pull/3170 Backport of #3138 to 9x. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17659) Implement basic authentication in Admin UI
[
https://issues.apache.org/jira/browse/SOLR-17659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17925405#comment-17925405
]
Christos Malliaridis commented on SOLR-17659:
-
I think I understand what you are referring to Gus. I believe this is clear
already, but allow me to repeat it once more to avoid any possible
misunderstanding: the new UI is just a binary file at the end of the day, not a
service that is run on the server where Solr is running. When the users access
the HTTP endpoint, they automatically download the file and run it in their
browser. Of course, this file is versioned and may leak the information of what
Solr version may be running because it is shipped with Solr (if the files are
not explicitly updated). It will obviously be >=v10.0, because the new UI does
not exist in versions prior to that. We would have the same "leak" anyway
regardless of the changes we introduce, even if we would introduce an auth
facade infront of the UI. So we would not improve the seuciry here.
The best way to protect Solr instances in such scenarios would be to disable
the UI completely and only allow clients like the standalone desktop app or
SolrJ to be used.
{quote}[...] use it to look for ways we have accidentally exposed information
or features that are not properly protected.
{quote}
I believe it would be way easier to analyze the endpoints from our source code,
rather than reverse-engineer the WebAssembly app that is a binary file to look
for unprotected information or features. The new UI does not know anything
about a Solr instance that is already leaked via the API. And I believe this is
a key difference compared to our current UI (I am not sure how the current UI
works though).
One could even host the new UI on a separate server and connect with any Solr
instance, like the desktop client of the new UI can do. So if we leak any kind
of information via our API already, and the new UI just uses that information,
then it should be addressed directly in the API level. One would not get more
information from what is already there.
{quote}[...] but we should do so in a structure suitable for all [use
cases|https://solr.apache.org/guide/solr/latest/deployment-guide/authentication-and-authorization-plugins.html#enabling-an-authentication-plugin].
{quote}
Yes, that is very important, and anyone implementing the auth should take into
account that credentials-based auth is not the only authentication option
possible. However, without implementing each use case, any structure that is
defined may not be properly evaluated until we implement all cases. There will
definitely be some kind of refactoring if the interfaces defined in the first
implementation do not work with all auth options available. But that is
acceptable and would be addressed once we reach that point of development.
{quote}An independent app that establishes auth and then redirects to the /ui
context
{quote}
That sounds very much like the concepts from OAuth. And from how I understand
your message, what you probably are looking for is "migrating" Solr into a
[resource server|https://www.oauth.com/oauth2-servers/the-resource-server/],
and allow an external app like Keycloak manage users, access etc. That would
move the responsibility of the auth stuff to a "separate app", improve the
security and reduce the complexity of Solr in general, and allow apps like the
new UI use a wide range of auth options by simply "redirecting" to the
Keycloack auth website. This would also handle all these redirects you
mentioned to the individual UIs. If that is what would address your
expectations, it would not be part of the new UI, but rather the entire Solr
project. And the new UI would not even be affected by that directly.
I hope I did not misunderstand any of your words.
> Implement basic authentication in Admin UI
> --
>
> Key: SOLR-17659
> URL: https://issues.apache.org/jira/browse/SOLR-17659
> Project: Solr
> Issue Type: New Feature
> Components: Admin UI
>Reporter: Christos Malliaridis
>Priority: Major
> Labels: new-ui, ui
>
> In the new UI one of the key features that is not implemented yet is user
> authentication. In order to secure and securily access Solr, the user should
> be able to authenticate against a Solr instance with basic credentials.
> h2. Task
> Implement basic user authentication (with credentials) according to the [new
> designs|https://www.figma.com/design/VdbEfcWQ8mirFNquBzbPk2/Apache-Solr-Admin-UI-v2-Concept?node-id=1190-388&t=vMgOa9QlzQZSdjLf-1].
> h2. Acceptance Criteria
> - The user can access a Solr instance that has user authentication enabled
> - The user can at least authenticate with credentials (basic auth)
> - The credentials form is displayed af
Re: [PR] SOLR-16427: Enable error-prone ClassInitializationDeadlock rule [solr]
dsmiley commented on PR #2881: URL: https://github.com/apache/solr/pull/2881#issuecomment-2646476270 Looking back at this a bit briefly. In general these plural form classes should be very unreferenced except when the registry itself is needed (should be rare). These classes should be considered `@lucene.internal` (we don't have a Solr variation yet) as we pursue ServiceProviderInterface. Instead of `DocRouters.DEFAULT`, add `DocRouter.getDefault()`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17649: Fix Json faceting on multivalue number types [solr]
dsmiley commented on PR #3158: URL: https://github.com/apache/solr/pull/3158#issuecomment-2646477023 Looking for additional code review since I contributed to this PR substantially. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Add NavigableObject.wrap. Deprecate MapWriter.EMPTY and MapWriterMap. [solr]
dsmiley commented on PR #3148: URL: https://github.com/apache/solr/pull/3148#issuecomment-2646517321 Will merge in a few days. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
