From aadc94a6e3887955ed08439d32e29fe31b622a45 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= <janhoy@apache.org>
Date: Fri, 5 Apr 2019 10:19:42 +0200
Subject: [PATCH] SOLR-12120: Harden AuditLoggerIntegrationTest, timing issues

---
 .../security/AuditLoggerIntegrationTest.java  | 26 ++++++++++++++-----
 .../security/CallbackAuditLoggerPlugin.java   |  2 +-
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
index 8f9e295cd9a..6ab0a7148d0 100644
--- a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
@@ -95,8 +95,8 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase {
   public void testSynchronous() throws Exception {
     setupCluster(false, 0, false, null);
     runAdminCommands();
+    waitForAuditEventCallbacks(3);
     assertAuditMetricsMinimums(testHarness.get().cluster, CallbackAuditLoggerPlugin.class.getSimpleName(), 3, 0);
-    testHarness.get().shutdownCluster();
     assertThreeAdminEvents();
   }
   
@@ -104,24 +104,34 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase {
   public void testAsync() throws Exception {
     setupCluster(true, 0, false, null);
     runAdminCommands();
+    waitForAuditEventCallbacks(3);
     assertAuditMetricsMinimums(testHarness.get().cluster, CallbackAuditLoggerPlugin.class.getSimpleName(), 3, 0);
-    testHarness.get().shutdownCluster();
     assertThreeAdminEvents();
   }
 
   @Test
-  public void testAsyncWithQueue() throws Exception {
+  public void testQueuedTimeMetric() throws Exception {
     setupCluster(true, 100, false, null);
     runAdminCommands();
+    waitForAuditEventCallbacks(3);
     assertAuditMetricsMinimums(testHarness.get().cluster, CallbackAuditLoggerPlugin.class.getSimpleName(), 3, 0);
     ArrayList<MetricRegistry> registries = getMetricsReigstries(testHarness.get().cluster);
     Timer timer = ((Timer) registries.get(0).getMetrics().get("SECURITY./auditlogging.CallbackAuditLoggerPlugin.queuedTime"));
     double meanTimeOnQueue = timer.getSnapshot().getMean() / 1000000; // Convert to ms
-    assertTrue(meanTimeOnQueue > 50);
+    assertTrue("Expecting mean time on queue >10ms, got " + meanTimeOnQueue, meanTimeOnQueue > 10);
+  }
+
+  @Test
+  public void testAsyncQueueDrain() throws Exception {
+    setupCluster(true, 100, false, null);
+    runAdminCommands();
+    assertTrue("Expecting <2 callbacks in buffer, was " + testHarness.get().receiver.getBuffer().size(),
+        testHarness.get().receiver.getBuffer().size() < 2); // Events still on queue
+    // We shutdown cluster while events are still in queue
     testHarness.get().shutdownCluster();
     assertThreeAdminEvents();
   }
-
+  
   @Test
   public void testMuteAdminListCollections() throws Exception {
     setupCluster(false, 0, false, "[ \"type:UNKNOWN\", [ \"path:/admin\", \"param:action=LIST\" ] ]");
@@ -208,11 +218,15 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase {
   }
 
   private void waitForAuditEventCallbacks(int number) throws InterruptedException {
+    waitForAuditEventCallbacks(number, 5);
+  }
+
+  private void waitForAuditEventCallbacks(int number, int timeoutSeconds) throws InterruptedException {
     CallbackReceiver receiver = testHarness.get().receiver;
     int count = 0;
     while(receiver.buffer.size() < number) { 
       Thread.sleep(100);
-      if (++count >= 30) fail("Failed waiting for " + number + " callbacks");
+      if (++count >= timeoutSeconds*10) fail("Failed waiting for " + number + " callbacks after " + timeoutSeconds + " seconds");
     }
   }
 
diff --git a/solr/core/src/test/org/apache/solr/security/CallbackAuditLoggerPlugin.java b/solr/core/src/test/org/apache/solr/security/CallbackAuditLoggerPlugin.java
index c0a829b2681..22cfbe2d9e0 100644
--- a/solr/core/src/test/org/apache/solr/security/CallbackAuditLoggerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/security/CallbackAuditLoggerPlugin.java
@@ -70,7 +70,7 @@ public class CallbackAuditLoggerPlugin extends AuditLoggerPlugin {
 
   @Override
   public void close() throws IOException { 
-    if (socket != null) socket.close();
     super.close();
+    if (socket != null) socket.close();
   }
 }