dsmiley commented on code in PR #975:
URL: https://github.com/apache/solr/pull/975#discussion_r962080721


##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -184,7 +185,7 @@
  * to make it work. When multi-core support was added to Solr way back in 
version 1.3, this class
  * was required so that the core functionality could be re-used multiple times.
  */
-public final class SolrCore implements SolrInfoBean, Closeable {
+public class SolrCore implements SolrInfoBean, Closeable {

Review Comment:
   Why not final anymore?



##########
solr/core/src/java/org/apache/solr/handler/configsets/ListConfigSetsAPI.java:
##########
@@ -16,34 +16,41 @@
  */
 package org.apache.solr.handler.configsets;
 
-import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
 import static 
org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-import java.util.List;
-import org.apache.solr.api.EndPoint;
-import org.apache.solr.client.solrj.SolrResponse;
-import org.apache.solr.cloud.OverseerSolrResponse;
-import org.apache.solr.common.util.NamedList;
+import javax.inject.Inject;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.HttpHeaders;
+import org.apache.solr.api.JerseyResource;
 import org.apache.solr.core.CoreContainer;
-import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.jersey.PermissionName;
 
 /**
  * V2 API for adding or updating a single file within a configset.
  *
  * <p>This API (GET /v2/cluster/configs) is analogous to the v1 
/admin/configs?action=LIST command.
  */
-public class ListConfigSetsAPI extends ConfigSetAPIBase {
+@Path("/cluster/configs")
+public class ListConfigSetsAPI extends JerseyResource {
+
+  @Context public HttpHeaders headers;
+
+  private final CoreContainer coreContainer;
+
+  @Inject
   public ListConfigSetsAPI(CoreContainer coreContainer) {
-    super(coreContainer);
+    this.coreContainer = coreContainer;
   }
 
-  @EndPoint(method = GET, path = "/cluster/configs", permission = 
CONFIG_READ_PERM)
-  public void listConfigSet(SolrQueryRequest req, SolrQueryResponse rsp) 
throws Exception {
-    final NamedList<Object> results = new NamedList<>();
-    List<String> configSetsList = configSetService.listConfigs();
-    results.add("configSets", configSetsList);
-    SolrResponse response = new OverseerSolrResponse(results);
-    rsp.getValues().addAll(response.getResponse());
+  @GET
+  @Produces({"application/json", "application/javabin"})
+  @PermissionName(CONFIG_READ_PERM)
+  public ListConfigsetsResponse listConfigSet() throws Exception {
+    final ListConfigsetsResponse response = new ListConfigsetsResponse();
+    response.configSets = coreContainer.getConfigSetService().listConfigs();
+    return response;

Review Comment:
   Let's have ListConfigSetsResponse be immutable with a constructor.  



##########
solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java:
##########
@@ -40,4 +44,41 @@ public static void flattenToCommaDelimitedString(
     final String flattenedStr = String.join(",", toFlatten);
     destination.put(newKey, flattenedStr);
   }
+
+  /**
+   * Convert a JacksonReflectMapWriter (typically a {@link
+   * org.apache.solr.jersey.SolrJerseyResponse}) into the NamedList on a 
SolrQueryResponse
+   *
+   * @param rsp the response to attach the resulting NamedList to
+   * @param mw the input object to be converted into a NamedList
+   * @param trimHeader should the 'responseHeader' portion of the response be 
added to the
+   *     NamedList, or should populating that header be left to code 
elsewhere. This value should
+   *     usually be 'false' when called from v2 code, and 'true' when called 
from v1 code.
+   */
+  public static void squashIntoSolrResponse(
+      SolrQueryResponse rsp, JacksonReflectMapWriter mw, boolean trimHeader) {
+    squashIntoNamedList(rsp.getValues(), mw, trimHeader);
+  }
+
+  public static void squashIntoSolrResponse(SolrQueryResponse rsp, 
JacksonReflectMapWriter mw) {
+    squashIntoSolrResponse(rsp, mw, false);
+  }
+
+  public static void squashIntoNamedList(
+      NamedList<Object> destination, JacksonReflectMapWriter mw) {
+    squashIntoNamedList(destination, mw, false);
+  }
+
+  // TODO Come up with a better approach (maybe change Responses to be based 
on some class that can
+  // natively do this
+  //  without the intermediate map(s)?)

Review Comment:
   I'll push a solution to this



##########
solr/core/src/java/org/apache/solr/api/JerseyResource.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.api;
+
+import org.apache.solr.jersey.CatchAllExceptionMapper;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.servlet.HttpSolrCall;
+
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.core.Context;
+import java.util.function.Supplier;
+
+import static 
org.apache.solr.jersey.RequestContextConstants.SOLR_JERSEY_RESPONSE_KEY;
+
+/**
+ * A marker parent type for all Jersey resource classes
+ */
+public class JerseyResource {
+
+    @Context
+    public ContainerRequestContext containerRequestContext;
+
+    /**
+     * Create an instance of the {@link SolrJerseyResponse} subclass; 
registering it with the Jersey request-context upon creation

Review Comment:
   nit: end the first sentence of any javadoc with a period. Really all 
sentences but especially the first as it acts as a summary.  Newlines are not 
honored in javadoc when rendered; so it's not a substitute for ending sentences 
with a period.



##########
solr/core/src/java/org/apache/solr/handler/configsets/ListConfigsetsResponse.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.handler.configsets;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+/** Response body POJO for the {@link ListConfigSetsAPI} resource. */
+public class ListConfigsetsResponse extends SolrJerseyResponse {

Review Comment:
   Can we just make this a static inner class of the API that needs it?  And 
make Immutable



##########
solr/core/src/java/org/apache/solr/jersey/MessageBodyWriters.java:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import static 
org.apache.solr.jersey.RequestContextConstants.SOLR_QUERY_REQUEST_KEY;
+import static 
org.apache.solr.jersey.RequestContextConstants.SOLR_QUERY_RESPONSE_KEY;
+import static 
org.apache.solr.response.QueryResponseWriter.CONTENT_TYPE_TEXT_UTF8;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ResourceContext;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.ext.MessageBodyWriter;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.BinaryResponseWriter;
+import org.apache.solr.response.CSVResponseWriter;
+import org.apache.solr.response.QueryResponseWriter;
+import org.apache.solr.response.QueryResponseWriterUtil;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.response.XMLResponseWriter;
+
+/**
+ * A collection of thin Jersey shims around Solr's existing {@link 
QueryResponseWriter} interface
+ */
+public class MessageBodyWriters {
+
+  // Jersey has a default MessageBodyWriter for JSON so we don't need to 
declare one here
+  // Which other response-writer formats are worth carrying forward into v2?
+
+  @Produces(MediaType.APPLICATION_XML)
+  public static class XmlMessageBodyWriter extends BaseMessageBodyWriter
+      implements MessageBodyWriter<JacksonReflectMapWriter> {
+    @Override
+    public QueryResponseWriter createResponseWriter() {
+      return new XMLResponseWriter();
+    }
+
+    @Override
+    public String getSupportedMediaType() {
+      return MediaType.APPLICATION_XML;
+    }
+  }
+
+  @Produces("application/javabin")

Review Comment:
   Jan suggested changing this



##########
solr/core/src/test/org/apache/solr/handler/configsets/package-info.java:
##########
@@ -0,0 +1,19 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/** Testing for the org.apache.solr.handler.configsets package. */
+package org.apache.solr.handler.configsets;

Review Comment:
   I hope we don't make it mandatory to provide package-info.java for *tests*



##########
solr/core/src/java/org/apache/solr/api/V2HttpCall.java:
##########
@@ -362,14 +363,15 @@ private void invokeJerseyRequest(CoreContainer cores, 
SolrCore core, Application
 
       // Set properties that may be used by Jersey filters downstream
       containerRequest.setProperty(SOLR_QUERY_REQUEST_KEY, solrReq);
-      
containerRequest.setProperty(SolrRequestAuthorizer.CORE_CONTAINER_PROP_NAME, 
cores);
-      
containerRequest.setProperty(SolrRequestAuthorizer.HTTP_SERVLET_REQ_PROP_NAME, 
req);
-      
containerRequest.setProperty(SolrRequestAuthorizer.REQUEST_TYPE_PROP_NAME, 
requestType);
-      
containerRequest.setProperty(SolrRequestAuthorizer.SOLR_PARAMS_PROP_NAME, 
queryParams);
-      
containerRequest.setProperty(SolrRequestAuthorizer.COLLECTION_LIST_PROP_NAME, 
collectionsList);
-      
containerRequest.setProperty(SolrRequestAuthorizer.HTTP_SERVLET_RSP_PROP_NAME, 
response);
+      containerRequest.setProperty(SOLR_QUERY_RESPONSE_KEY, rsp);

Review Comment:
   I agree; it's consistency.  FWIW I like the code being fully qualified 
because it shows where the constant is coming from.



##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -180,6 +185,17 @@ public CoreLoadFailure(CoreDescriptor cd, Exception 
loadFailure) {
   private volatile PluginBag<SolrRequestHandler> containerHandlers =
       new PluginBag<>(SolrRequestHandler.class, null);
 
+  private volatile ResourceConfig config;

Review Comment:
   Please name this variable `jerseyResourceConfig`.  



##########
solr/core/src/java/org/apache/solr/api/JerseyResource.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.api;
+
+import org.apache.solr.jersey.CatchAllExceptionMapper;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.servlet.HttpSolrCall;
+
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.core.Context;
+import java.util.function.Supplier;
+
+import static 
org.apache.solr.jersey.RequestContextConstants.SOLR_JERSEY_RESPONSE_KEY;
+
+/**
+ * A marker parent type for all Jersey resource classes
+ */
+public class JerseyResource {
+
+    @Context
+    public ContainerRequestContext containerRequestContext;
+
+    /**
+     * Create an instance of the {@link SolrJerseyResponse} subclass; 
registering it with the Jersey request-context upon creation
+     *
+     * This utility method primarily exists to allow Jersey resources to 
return error responses that match those
+     * returned by Solr's v1 APIs.
+     *
+     * When a severe-enough exception halts a v1 request, Solr generates a 
summary of the error and attaches it to the
+     * {@link org.apache.solr.response.SolrQueryResponse} given to the request 
handler.  This SolrQueryResponse may
+     * already hold some portion of the normal "success" response for that API.
+     *
+     * The JAX-RS framework isn't well suited to mimicking responses of this 
sort, as the "response" from a Jersey resource
+     * is its return value (instead of a passed-in value that gets modified).  
This utility works around this limitation
+     * by attaching the eventual return value of a JerseyResource to the 
context associated with the Jersey
+     * request.  This allows partially-constructed responses to be accessed 
later in the case of an exception.
+     *
+     * In order to instantiate arbitrary SolrJerseyResponse subclasses, this 
utility uses reflection to find and invoke
+     * the first (no-arg) constructor for the specified type.  
SolrJerseyResponse subclasses without a no-arg constructor
+     * can be instantiated and registered using {@link 
#instantiateJerseyResponse(Supplier)}
+     *
+     * @param clazz the SolrJerseyResponse class to instantiate and register
+     *
+     * @see CatchAllExceptionMapper
+     * @see HttpSolrCall#call()
+     */
+    @SuppressWarnings("unchecked")
+    protected <T extends SolrJerseyResponse> T 
instantiateJerseyResponse(Class<T> clazz) {
+        return instantiateJerseyResponse(() -> {
+            try {
+                return (T) clazz.getConstructors()[0].newInstance();
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        });
+    }
+
+    /**
+     * Create an instance of the {@link SolrJerseyResponse} subclass; 
registering it with the Jersey request-context upon creation

Review Comment:
   Again; misses period.



##########
solr/core/src/java/org/apache/solr/api/V2HttpCall.java:
##########
@@ -388,33 +335,65 @@ public CompositeApi add(Api api) {
     }
   }
 
+  // We don't rely on any of Jersey's authc/z features so we pass in this 
empty context for
+  // all requests.
+  public static final SecurityContext DEFAULT_SECURITY_CONTEXT = new 
SecurityContext() {
+    public boolean isUserInRole(String role) { return false; }
+    public boolean isSecure() { return false; }
+    public Principal getUserPrincipal() { return null; }
+    public String getAuthenticationScheme() { return null; }
+  };
+
+  private void invokeJerseyRequest(CoreContainer cores, SolrCore core, 
ApplicationHandler jerseyHandler, SolrQueryResponse rsp) {
+    try {
+      final ContainerRequest containerRequest = 
ContainerRequestUtils.createContainerRequest(req, response, 
jerseyHandler.getConfiguration());
+
+      // Set properties that may be used by Jersey filters downstream
+      containerRequest.setProperty(SOLR_QUERY_REQUEST_KEY, solrReq);
+      containerRequest.setProperty(SOLR_QUERY_RESPONSE_KEY, rsp);
+      containerRequest.setProperty(RequestContextConstants.CORE_CONTAINER_KEY, 
cores);

Review Comment:
   `RequestContextConstants` holds nothing but "keys", so we don't need a 
suffix of `_KEY` on each one.



##########
solr/core/src/java/org/apache/solr/api/V2HttpCall.java:
##########
@@ -273,7 +278,7 @@ public static Api getApiInfo(
     }
 
     if (api == null) {
-      return getSubPathApi(requestHandlers, path, fullPath, new 
CompositeApi(null));

Review Comment:
   What became of this getSubPathApi?



##########
solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java:
##########
@@ -215,54 +215,59 @@ public void handleRequest(SolrQueryRequest req, 
SolrQueryResponse rsp) {
         }
       }
     } catch (Exception e) {
-      if (req.getCore() != null) {
-        boolean isTragic = 
req.getCoreContainer().checkTragicException(req.getCore());
-        if (isTragic) {
-          if (e instanceof SolrException) {
-            // Tragic exceptions should always throw a server error
-            assert ((SolrException) e).code() == 500;
-          } else {
-            // wrap it in a solr exception
-            e = new SolrException(SolrException.ErrorCode.SERVER_ERROR, 
e.getMessage(), e);
-          }
-        }
-      }
-      boolean incrementErrors = true;
-      boolean isServerError = true;
-      if (e instanceof SolrException) {
-        SolrException se = (SolrException) e;
-        if (se.code() == SolrException.ErrorCode.CONFLICT.code) {
-          incrementErrors = false;
-        } else if (se.code() >= 400 && se.code() < 500) {
-          isServerError = false;
-        }
-      } else {
-        if (e instanceof SyntaxError) {
-          isServerError = false;
-          e = new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
-        }
-      }
-
+      e = normalizeReceivedException(req, e);
+      processErrorMetricsOnException(e, metrics);
       rsp.setException(e);
+    } finally {
+      long elapsed = timer.stop();
+      metrics.totalTime.inc(elapsed);
+    }
+  }
 
-      if (incrementErrors) {
-        SolrException.log(log, e);
+  public static void processErrorMetricsOnException(Exception e, 
HandlerMetrics metrics) {
+    boolean isClientError = false;
+    if (e instanceof SolrException) {
+      final SolrException se = (SolrException) e;
+      if (se.code() == SolrException.ErrorCode.CONFLICT.code) {
+        return;
+      } else if (se.code() >= 400 && se.code() < 500) {
+        isClientError = true;
+      }
+    }
+
+    SolrException.log(log, e);
+    metrics.numErrors.mark();
+    if (isClientError) {
+      metrics.numClientErrors.mark();
+    } else {
+      metrics.numServerErrors.mark();
+    }
+  }
 
-        metrics.numErrors.mark();
-        if (isServerError) {
-          metrics.numServerErrors.mark();
+  public static Exception normalizeReceivedException(SolrQueryRequest req, 
Exception e) {
+    if (req.getCore() != null) {
+      assert req.getCoreContainer() != null;
+      boolean isTragic = 
req.getCoreContainer().checkTragicException(req.getCore());
+      if (isTragic) {
+        if (e instanceof SolrException) {
+          // Tragic exceptions should always throw a server error
+          assert ((SolrException) e).code() == 500;

Review Comment:
   equals is too strict; I think >= 500 & < 600.



##########
solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java:
##########
@@ -192,6 +200,28 @@ public Category getCategory() {
     return Category.ADMIN;
   }
 
+  public Boolean registerV2() {
+    return true;
+  }
+
+  @Override
+  public Collection<Api> getApis() {
+    final List<Api> apis = new ArrayList<>();
+    apis.addAll(AnnotatedApi.getApis(new CreateConfigSetAPI(coreContainer)));
+    apis.addAll(AnnotatedApi.getApis(new DeleteConfigSetAPI(coreContainer)));
+    apis.addAll(AnnotatedApi.getApis(new UploadConfigSetAPI(coreContainer)));
+    apis.addAll(AnnotatedApi.getApis(new 
UploadConfigSetFileAPI(coreContainer)));
+
+    return apis;
+  }
+
+  @Override
+  public Collection<Class<? extends JerseyResource>> getJerseyResources() {

Review Comment:
   List.of



##########
solr/core/src/java/org/apache/solr/core/PluginBag.java:
##########
@@ -213,6 +237,24 @@ public PluginHolder<T> put(String name, PluginHolder<T> 
plugin) {
                 apiBag.register(api, nameSubstitutes);
               }
             }
+
+            // TODO Should we use <requestHandler name="/blah"> to override 
the path that each
+            //  resource registers under?
+            Collection<Class<? extends JerseyResource>> jerseyApis =
+                apiSupport.getJerseyResources();
+            if (!CollectionUtils.isEmpty(jerseyApis)) {
+              for (Class<? extends JerseyResource> jerseyClazz : jerseyApis) {
+                if (log.isDebugEnabled()) {
+                  log.debug("Registering jersey resource class: {}", 
jerseyClazz.getName());
+                }
+                jerseyResources.register(jerseyClazz);
+                // See MetricsBeanFactory javadocs for a better understanding 
of this resource->RH
+                // mapping
+                if (inst instanceof RequestHandlerBase) {

Review Comment:
   This reference to RequestHandlerBase is a smell to me.  There is a typical 
general pattern where an interface exists (SolrRequestInfo) and a base 
implementation (RequestHandlerBase) that is an implementation detail and 
ideally not mandatory.  So I don't think we should refer to RequestHandlerBase 
unless it's to extend it.  Likewise I got wind of this smell when you made some 
RHB methods public that were protected; it's suspicious.



##########
solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java:
##########
@@ -192,6 +200,28 @@ public Category getCategory() {
     return Category.ADMIN;
   }
 
+  public Boolean registerV2() {
+    return true;
+  }
+
+  @Override
+  public Collection<Api> getApis() {

Review Comment:
   List.of



##########
solr/core/src/java/org/apache/solr/core/PluginBag.java:
##########
@@ -490,4 +532,8 @@ public Api v2lookup(String path, String method, Map<String, 
String> parts) {
   public ApiBag getApiBag() {
     return apiBag;
   }
+
+  public ResourceConfig getJerseyEndpoints() {

Review Comment:
   It's curious to me to see getXXXX and the type doesn't have an obvious 
correlation to XXXX.  Can you explain?  In CoreContainer you called a similar 
method just getResourceConfig.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/GetSchemaNameResponse.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.handler.admin.api;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+public class GetSchemaNameResponse extends SolrJerseyResponse {

Review Comment:
   This could even be an inner class of the handler that uses it?



##########
solr/core/src/java/org/apache/solr/api/JerseyResource.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.api;
+
+import static 
org.apache.solr.jersey.RequestContextConstants.SOLR_JERSEY_RESPONSE_KEY;
+
+import java.util.function.Supplier;
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.core.Context;
+import org.apache.solr.jersey.CatchAllExceptionMapper;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.servlet.HttpSolrCall;
+
+/** A marker parent type for all Jersey resource classes */

Review Comment:
   And what is a "Jersey resource class"?  I'm new to this.  Maybe say it's for 
"request handlers" in the V3 API?



##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -1955,6 +1958,10 @@ public PluginBag<SolrRequestHandler> 
getRequestHandlers() {
     return reqHandlers.handlers;
   }
 
+  public ApplicationHandler getApplicationHandler() {

Review Comment:
   Use "Jersey" in the name please.  Same with the field name.



##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -180,6 +185,17 @@ public CoreLoadFailure(CoreDescriptor cd, Exception 
loadFailure) {
   private volatile PluginBag<SolrRequestHandler> containerHandlers =
       new PluginBag<>(SolrRequestHandler.class, null);
 
+  private volatile ResourceConfig config;
+  private volatile ApplicationHandler appHandler;

Review Comment:
   Please name this `jerseyAppHandler`.
   
   The point is to make it clear where we have our "Jersey" stuff in existing 
Solr classes with tons of Solr things so that we can clearly spot major _other_ 
APIs.  Names like "ResourceConfig" and "ApplicationHandler" are so generic 
sounding that it's not obvious it's related to Jersey unless you already know 
Jersey well.
   
   Similarly the getters could be named accordingly at your discretion.



##########
solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java:
##########
@@ -136,7 +143,8 @@ public SolrParams getParams() {
         }
         break;
       case LIST:
-        new ListConfigSetsAPI(coreContainer).listConfigSet(req, rsp);
+        final ListConfigSetsAPI listConfigSetsAPI = new 
ListConfigSetsAPI(coreContainer);
+        V2ApiUtils.squashIntoSolrResponse(rsp, 
listConfigSetsAPI.listConfigSet(), true);

Review Comment:
   The final boolean is harder to judge reading the code; I think it would be 
clearer if it was called squashIntoSolrResponseWithoutHeader even though it's a 
long name.



##########
solr/core/src/java/org/apache/solr/jersey/ApplicationEventLogger.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import java.lang.invoke.MethodHandles;
+import org.glassfish.jersey.server.monitoring.ApplicationEvent;
+import org.glassfish.jersey.server.monitoring.ApplicationEventListener;
+import org.glassfish.jersey.server.monitoring.RequestEvent;
+import org.glassfish.jersey.server.monitoring.RequestEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Logs out application-level information useful for troubleshooting Jersey 
development.
+ *
+ * @see RequestEventLogger
+ */
+public class ApplicationEventLogger implements ApplicationEventListener {
+  private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  private volatile long requestCount = 0;
+
+  @Override
+  public void onEvent(ApplicationEvent event) {
+    if (log.isInfoEnabled()) {
+      log.info("Received ApplicationEvent {}", event.getType());
+    }
+  }
+
+  @Override
+  public RequestEventListener onRequest(RequestEvent requestEvent) {
+    requestCount++;
+    log.info("Starting Jersey request {}", requestCount);

Review Comment:
   Another INFO level that should probably simply be removed



##########
solr/core/src/java/org/apache/solr/handler/admin/api/GetSchemaNameResponse.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.handler.admin.api;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+public class GetSchemaNameResponse extends SolrJerseyResponse {
+  @JsonProperty("name")
+  public String name;

Review Comment:
   Instead of a mutable class; can't this be a simple immutable object with a 
constructor?



##########
solr/core/src/java/org/apache/solr/handler/api/ApiRegistrar.java:
##########
@@ -79,12 +73,4 @@ public static void registerShardApis(ApiBag apiBag, 
CollectionsHandler collectio
     // here for simplicity.
     apiBag.registerObject(new DeleteReplicaAPI(collectionsHandler));
   }
-
-  public static void registerConfigsetApis(ApiBag apiBag, CoreContainer 
container) {

Review Comment:
   These are no longer registered in ApiBag?  I don't see that the code merely 
moved.



##########
solr/core/src/java/org/apache/solr/jersey/MetricBeanFactory.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import org.apache.solr.core.PluginBag;
+import org.glassfish.hk2.api.Factory;
+
+/**
+ * Factory to inject JerseyMetricsLookupRegistry instances into Jersey 
resources and filters.
+ *
+ * <p>Currently, Jersey resources that have a corresponding v1 API produce the 
same metrics as their
+ * v1 equivalent and rely on the v1 requestHandler instance to do so. Solr 
facilitates this by
+ * building a map of the Jersey resource to requestHandler mapping (a {@link
+ * org.apache.solr.core.PluginBag.JerseyMetricsLookupRegistry}), and injecting 
it into the pre- and
+ * post- Jersey filters that handle metrics.
+ *
+ * <p>This isn't ideal, as requestHandler's don't really "fit" conceptually 
here. But it's
+ * unavoidable while we want our v2 APIs to exactly match the metrics produced 
by v1 calls.

Review Comment:
   If we didn't retain this exact metrics match with V1, what might a metric 
look like before & after?



##########
solr/core/src/test/org/apache/solr/handler/configsets/ListConfigSetsAPITest.java:
##########
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.handler.configsets;
+
+import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import javax.inject.Singleton;
+import javax.ws.rs.core.Application;
+import javax.ws.rs.core.Response;
+import org.apache.solr.client.solrj.response.ConfigSetAdminResponse;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.ConfigSetService;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.jersey.CoreContainerFactory;
+import org.apache.solr.jersey.SolrJacksonMapper;
+import org.glassfish.hk2.utilities.binding.AbstractBinder;
+import org.glassfish.jersey.server.ResourceConfig;
+import org.glassfish.jersey.test.JerseyTest;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class ListConfigSetsAPITest extends JerseyTest {
+
+  private CoreContainer mockCoreContainer;
+
+  @BeforeClass
+  public static void ensureWorkingMockito() {
+    assumeWorkingMockito();
+  }
+
+  @Override
+  protected Application configure() {
+    resetMocks();
+    final ResourceConfig config = new ResourceConfig();
+    config.register(ListConfigSetsAPI.class);
+    config.register(SolrJacksonMapper.class);
+    config.register(
+        new AbstractBinder() {
+          @Override
+          protected void configure() {
+            bindFactory(new CoreContainerFactory(mockCoreContainer))
+                .to(CoreContainer.class)
+                .in(Singleton.class);
+          }
+        });
+
+    return config;
+  }
+
+  private void resetMocks() {
+    mockCoreContainer = mock(CoreContainer.class);
+  }
+
+  @Test
+  public void testSuccessfulListConfigsetsRaw() throws Exception {
+    final String expectedJson =
+        
"{\"responseHeader\":{\"status\":0,\"QTime\":0},\"configSets\":[\"cs1\",\"cs2\"]}";
+    final ConfigSetService configSetService = mock(ConfigSetService.class);
+    when(mockCoreContainer.getConfigSetService()).thenReturn(configSetService);
+    when(configSetService.listConfigs()).thenReturn(List.of("cs1", "cs2"));
+
+    final Response response = target("/cluster/configs").request().get();
+    final String jsonBody = response.readEntity(String.class);
+
+    assertEquals(200, response.getStatus());
+    assertEquals("application/json", 
response.getHeaders().getFirst("Content-type"));
+    assertEquals(1, 1);
+    assertEquals(
+        expectedJson,
+        
"{\"responseHeader\":{\"status\":0,\"QTime\":0},\"configSets\":[\"cs1\",\"cs2\"]}");
+  }
+
+  @Test
+  public void testSuccessfulListConfigsetsTyped() throws Exception {
+    final ConfigSetService configSetService = mock(ConfigSetService.class);
+    when(mockCoreContainer.getConfigSetService()).thenReturn(configSetService);
+    when(configSetService.listConfigs()).thenReturn(List.of("cs1", "cs2"));
+
+    final ListConfigsetsResponse response =
+        target("/cluster/configs").request().get(ListConfigsetsResponse.class);
+
+    assertNotNull(response.configSets);
+    assertNull(response.error);
+    assertEquals(2, response.configSets.size());
+    assertTrue(response.configSets.contains("cs1"));
+    assertTrue(response.configSets.contains("cs2"));
+  }
+
+  /**
+   * Test the v2 to v1 response mapping for /cluster/configs
+   *
+   * <p>{@link org.apache.solr.handler.admin.ConfigSetsHandler} uses {@link 
ListConfigSetsAPI} (and
+   * its response class {@link ListConfigsetsResponse}) internally to serve 
the v1 version of this
+   * functionality. So it's important to make sure that the v2 response stays 
compatible with SolrJ
+   * - both because that's important in its own right and because that ensures 
we haven't
+   * accidentally changed the v1 response format.
+   */
+  @Test
+  public void testListConfigsetsV1Compatibility() throws Exception {

Review Comment:
   IMO the best test for v1 or v-whatever compatibility would be high level -- 
test the JSON or XML.  Such tests are very easy to understand and thus 
high-confidence as well (i.e. is whatever _really_ tested).  Then we can play 
with class internals without tests acting as quick-sand for change (my beef 
with many unit tests).  Why test internals like this?



##########
solr/core/src/java/org/apache/solr/jersey/RequestContextConstants.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import com.codahale.metrics.Timer;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.container.ContainerRequestContext;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.security.AuthorizationContext;
+
+/**
+ * Keys used to store and retrieve values from the Jersey request context.
+ *
+ * <p>Properties are generally set in V2HttpCall's 'invokeJerseyRequest' and 
retrieved in individual
+ * {@link javax.ws.rs.container.ContainerRequestFilter}s using {@link
+ * ContainerRequestContext#getProperty(String)}
+ */
+public class RequestContextConstants {

Review Comment:
   Interfaces are nice to hold constants -- you can omit the "public static 
final" before each.  I added a several relating to cluster state like 
Slice.SliceStateProps recently.



##########
versions.props:
##########
@@ -56,6 +57,8 @@ org.bitbucket.b_c:jose4j=0.7.9
 org.carrot2:carrot2-core=4.4.2
 org.codehaus.woodstox:stax2-api=4.2.1
 org.eclipse.jetty*:*=9.4.44.v20210927
+org.glassfish.hk2*:*=2.6.1
+org.glassfish.jersey*:*=2.35

Review Comment:
   there is a Jersey 2.36



##########
solr/core/src/java/org/apache/solr/jersey/SolrJerseyResponse.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ * Base response-body POJO to be used by Jersey resources.
+ *
+ * <p>Contains fields common to all Solr API responses, particularly the 
'responseHeader' and
+ * 'error' fields.
+ */
+public class SolrJerseyResponse implements JacksonReflectMapWriter {
+
+  @JsonProperty("responseHeader")
+  public ResponseHeader responseHeader = new ResponseHeader();
+
+  @JsonProperty("error")
+  public ErrorInfo error;
+
+  public static class ResponseHeader implements JacksonReflectMapWriter {

Review Comment:
   Can't there be more; like doesn't echoParams stuff show up under here too?  
I'm a little concerned that SolrJerseyResponse may be a bit rigid in structure 
when Solr is rather flexible.



##########
solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java:
##########
@@ -215,54 +215,59 @@ public void handleRequest(SolrQueryRequest req, 
SolrQueryResponse rsp) {
         }
       }
     } catch (Exception e) {
-      if (req.getCore() != null) {
-        boolean isTragic = 
req.getCoreContainer().checkTragicException(req.getCore());
-        if (isTragic) {
-          if (e instanceof SolrException) {
-            // Tragic exceptions should always throw a server error
-            assert ((SolrException) e).code() == 500;
-          } else {
-            // wrap it in a solr exception
-            e = new SolrException(SolrException.ErrorCode.SERVER_ERROR, 
e.getMessage(), e);
-          }
-        }
-      }
-      boolean incrementErrors = true;
-      boolean isServerError = true;
-      if (e instanceof SolrException) {
-        SolrException se = (SolrException) e;
-        if (se.code() == SolrException.ErrorCode.CONFLICT.code) {
-          incrementErrors = false;
-        } else if (se.code() >= 400 && se.code() < 500) {
-          isServerError = false;
-        }
-      } else {
-        if (e instanceof SyntaxError) {
-          isServerError = false;
-          e = new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
-        }
-      }
-
+      e = normalizeReceivedException(req, e);
+      processErrorMetricsOnException(e, metrics);
       rsp.setException(e);
+    } finally {
+      long elapsed = timer.stop();
+      metrics.totalTime.inc(elapsed);
+    }
+  }
 
-      if (incrementErrors) {
-        SolrException.log(log, e);
+  public static void processErrorMetricsOnException(Exception e, 
HandlerMetrics metrics) {
+    boolean isClientError = false;
+    if (e instanceof SolrException) {
+      final SolrException se = (SolrException) e;
+      if (se.code() == SolrException.ErrorCode.CONFLICT.code) {
+        return;
+      } else if (se.code() >= 400 && se.code() < 500) {
+        isClientError = true;
+      }
+    }
+
+    SolrException.log(log, e);
+    metrics.numErrors.mark();
+    if (isClientError) {
+      metrics.numClientErrors.mark();
+    } else {
+      metrics.numServerErrors.mark();
+    }
+  }
 
-        metrics.numErrors.mark();
-        if (isServerError) {
-          metrics.numServerErrors.mark();
+  public static Exception normalizeReceivedException(SolrQueryRequest req, 
Exception e) {
+    if (req.getCore() != null) {
+      assert req.getCoreContainer() != null;
+      boolean isTragic = 
req.getCoreContainer().checkTragicException(req.getCore());
+      if (isTragic) {
+        if (e instanceof SolrException) {
+          // Tragic exceptions should always throw a server error
+          assert ((SolrException) e).code() == 500;

Review Comment:
   better yet, add this as a SolrException method.



##########
solr/core/src/java/org/apache/solr/jersey/JerseyApplications.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import java.util.Map;
+import javax.inject.Singleton;
+import org.apache.solr.core.PluginBag;
+import org.apache.solr.core.SolrCore;
+import org.glassfish.hk2.utilities.binding.AbstractBinder;
+import org.glassfish.jersey.server.ResourceConfig;
+
+/**
+ * JAX-RS "application" configurations for Solr's {@link 
org.apache.solr.core.CoreContainer} and
+ * {@link SolrCore} instances
+ */
+public class JerseyApplications {
+
+  public static class CoreContainerApp extends ResourceConfig {
+    public CoreContainerApp(PluginBag.JerseyMetricsLookupRegistry 
beanRegistry) {
+      super();
+
+      // Authentication and authorization
+      register(SolrRequestAuthorizer.class);
+
+      // Request and response serialization/deserialization
+      // TODO: could these be singletons to save per-request object creations?
+      register(MessageBodyWriters.JavabinMessageBodyWriter.class);
+      register(MessageBodyWriters.XmlMessageBodyWriter.class);
+      register(MessageBodyWriters.CsvMessageBodyWriter.class);
+      register(SolrJacksonMapper.class);
+
+      // Request lifecycle logic
+      register(CatchAllExceptionMapper.class);
+      register(PreRequestMetricsFilter.class);
+      register(PostRequestMetricsFilter.class);

Review Comment:
   I think these two classes could be combined and it'd be clearer for them 
since they interact (pre & post for same topic).



-- 
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

Reply via email to