From 615cc38b18105951e0c12ec8a9851c0baee13d19 Mon Sep 17 00:00:00 2001
From: Steve Rowe <sarowe@apache.org>
Date: Wed, 26 Jul 2017 12:21:02 -0400
Subject: [PATCH] SOLR-11151: SolrInfoMBeanHandler.getDiff() ADD case
 non-functional: NPE when a bean value goes from null -> non-null

---
 solr/CHANGES.txt                              |  3 ++
 .../handler/admin/SolrInfoMBeanHandler.java   | 14 ++++----
 .../solr/handler/admin/MBeansHandlerTest.java | 34 +++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3d6c5a12747..821f9facf51 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -391,6 +391,9 @@ Bug Fixes
 
 * SOLR-11130: V2Request in SolrJ should return the correct collection name so that the request is forwarded to the
   correct node (noble)
+  
+* SOLR-11151: SolrInfoMBeanHandler.getDiff() ADD case non-functional: NPE when a bean value goes from null -> non-null.
+  (Steve Rowe)
 
 Optimizations
 ----------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SolrInfoMBeanHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/SolrInfoMBeanHandler.java
index 4faa466f4ee..f9a3c053cdc 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/SolrInfoMBeanHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/SolrInfoMBeanHandler.java
@@ -209,14 +209,16 @@ public class SolrInfoMBeanHandler extends RequestHandlerBase {
     for(int i=0; i<ref.size(); i++) {
       String name = ref.getName(i);
       Object r = ref.getVal(i);
-      Object n = now.remove(name);
-      if(n == null) {
-        if(r!=null) {
-          out.add("REMOVE "+name, r);
+      Object n = now.get(name);
+      if (n == null) {
+        if (r != null) {
+          out.add("REMOVE " + name, r);
+          now.remove(name);
         }
       }
-      else {
-        out.add(name, diffObject(r,n));
+      else if (r != null) {
+        out.add(name, diffObject(r, n));
+        now.remove(name);
       }
     }
     
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java
index 40d7821efed..e47dae5fcd2 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java
@@ -16,8 +16,13 @@
  */
 package org.apache.solr.handler.admin;
 
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+import java.lang.management.ManagementFactory;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -26,6 +31,12 @@ import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.ContentStream;
 import org.apache.solr.common.util.ContentStreamBase;
 import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.SolrCoreMetricManager;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricReporter;
+import org.apache.solr.metrics.reporters.JmxObjectNameFactory;
+import org.apache.solr.metrics.reporters.SolrJmxReporter;
 import org.apache.solr.request.LocalSolrQueryRequest;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -82,6 +93,29 @@ public class MBeansHandlerTest extends SolrTestCaseJ4 {
     assertNotNull( nl.get("ADMIN").get("org.apache.solr.handler.admin.CollectionsHandler"));
   }
 
+  @Test
+  public void testAddedMBeanDiff() throws Exception {
+    String xml = h.query(req(
+        CommonParams.QT,"/admin/mbeans",
+        "stats","true",
+        CommonParams.WT,"xml"
+    ));
+
+    // Artificially convert a long value to a null, to trigger the ADD case in SolrInfoMBeanHandler.diffObject()
+    xml = xml.replaceFirst("<long\\s+(name\\s*=\\s*\"ADMIN./admin/mbeans.totalTime\"\\s*)>[^<]*</long>", "<null $1/>");
+
+    LocalSolrQueryRequest req = lrf.makeRequest(
+        CommonParams.QT,"/admin/mbeans",
+        "stats","true",
+        CommonParams.WT,"xml",
+        "diff","true");
+    req.setContentStreams(Collections.singletonList(new ContentStreamBase.StringStream(xml)));
+    xml = h.query(req);
+
+    NamedList<NamedList<NamedList<Object>>> nl = SolrInfoMBeanHandler.fromXML(xml);
+    assertNotNull(((NamedList)nl.get("ADMIN").get("/admin/mbeans").get("stats")).get("ADD ADMIN./admin/mbeans.totalTime"));
+  }
+
   @Test
   public void testXMLDiffWithExternalEntity() throws Exception {
     String file = getFile("mailing_lists.pdf").toURI().toASCIIString();