You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/06 05:01:56 UTC

[GitHub] [solr] dsmiley commented on a diff in pull request #975: Allow JAX-RS v2 API definition

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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org