SOLR-6179: Better strategy for handling empty managed data to avoid spurious warning messages in the logs.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1607102 13f79535-47bb-0310-9956-ffa450edef68

solr/core/src/java/org/apache/solr/rest/ManagedResource.java

@@ -82,6 +82,7 @@ public abstract class ManagedResource {
    */
   public void loadManagedDataAndNotify(List<ManagedResourceObserver> observers) 
       throws SolrException {
+
     // load managed data from storage
     reloadFromStorage();
     
@@ -89,7 +90,7 @@ public abstract class ManagedResource {
     // also, as most analysis components will alter the initArgs it is processes them
     // we need to clone the managed initArgs
     notifyObserversDuringInit(managedInitArgs, observers);
-
+    
     // some basic date tracking around when the data was initialized and updated
     initializedOn = new Date();
     lastUpdateSinceInitialization = null;    
@@ -105,10 +106,8 @@ public abstract class ManagedResource {
   protected void notifyObserversDuringInit(NamedList<?> args, List<ManagedResourceObserver> observers)
       throws SolrException {
 
-    if (observers == null || observers.isEmpty()) {
-      log.warn("No registered observers for {}", getResourceId());
+    if (observers == null || observers.isEmpty())
       return;
-    }
     
     for (ManagedResourceObserver observer : observers) {
       // clone the args for each observer as some components
@@ -163,7 +162,7 @@ public abstract class ManagedResource {
       return false;
     }
     boolean madeChanges = false;
-    if ( ! managedInitArgs.equals(updatedArgs)) {
+    if (!managedInitArgs.equals(updatedArgs)) {
       managedInitArgs = (NamedList<Object>)updatedArgs.clone();
       madeChanges = true;
     }
@@ -251,14 +250,17 @@ public abstract class ManagedResource {
     } catch (Throwable storeErr) {
       
       // store failed, so try to reset the state of this object by reloading
-      // from storage and then failing the store request
-      try {
-        reloadFromStorage();
-      } catch (Exception reloadExc) {
-        // note: the data we're managing now remains in a dubious state
-        // however the text analysis component remains unaffected 
-        // (at least until core reload)
-        log.error("Failed to load stop words from storage due to: "+reloadExc);
+      // from storage and then failing the store request, but only do that
+      // if we've successfully initialized before
+      if (initializedOn != null) {
+        try {
+          reloadFromStorage();
+        } catch (Exception reloadExc) {
+          // note: the data we're managing now remains in a dubious state
+          // however the text analysis component remains unaffected 
+          // (at least until core reload)
+          log.error("Failed to load stop words from storage due to: "+reloadExc);
+        }
       }
       
       String errMsg = String.format(Locale.ROOT,
@@ -273,6 +275,9 @@ public abstract class ManagedResource {
    * Returns this resource's initialization timestamp.
    */
   public String getInitializedOn() {
+    if (initializedOn == null)
+      return null;
+    
     StringBuilder dateBuf = new StringBuilder();
     try {
       DateUtil.formatDate(initializedOn, null, dateBuf);
@@ -315,7 +320,10 @@ public abstract class ManagedResource {
     toStore.put(INIT_ARGS_JSON_FIELD, convertNamedListToMap(managedInitArgs));
     
     // report important dates when data was init'd / updated
-    toStore.put(INITIALIZED_ON_JSON_FIELD, getInitializedOn());
+    String initializedOnStr = getInitializedOn();
+    if (initializedOnStr != null) {
+      toStore.put(INITIALIZED_ON_JSON_FIELD, initializedOnStr);
+    }
     
     // if the managed data has been updated since initialization (ie. it's dirty)
     // return that in the response as well ... which gives a good hint that the

solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java

@@ -135,10 +135,11 @@ public abstract class ManagedResourceStorage {
     
     @Override
     public void configure(SolrResourceLoader loader, NamedList<String> initArgs) throws SolrException {
-      String storageDirArg = initArgs.get("storageDir");
+      String storageDirArg = initArgs.get(STORAGE_DIR_INIT_ARG);
       
       if (storageDirArg == null || storageDirArg.trim().length() == 0)
-        throw new IllegalArgumentException("Required configuration parameter 'storageDir' not provided!");
+        throw new IllegalArgumentException("Required configuration parameter '"+
+           STORAGE_DIR_INIT_ARG+"' not provided!");
       
       File dir = new File(storageDirArg);
       if (!dir.isDirectory())

solr/core/src/java/org/apache/solr/rest/RestManager.java

@@ -452,7 +452,11 @@ public class RestManager {
         throws SolrException {
       
       if (managedData == null) {
-        return; // this is OK, just means there are no stored registrations
+        // this is OK, just means there are no stored registrations
+        // storing an empty list is safe and avoid future warnings about
+        // the data not existing
+        storeManagedData(new ArrayList<Map<String,String>>(0));
+        return;
       }
       
       List<Object> managedList = (List<Object>)managedData;

solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedWordSetResource.java

@@ -16,9 +16,11 @@ package org.apache.solr.rest.schema.analysis;
  * limitations under the License.
  */
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
@@ -104,6 +106,8 @@ public class ManagedWordSetResource extends ManagedResource
       } else {
         managedWords.addAll(wordList);        
       }
+    } else {
+      storeManagedData(new ArrayList<String>(0)); // stores an empty word set      
     }
     
     log.info("Loaded "+managedWords.size()+" words for "+getResourceId());      

solr/core/src/test/org/apache/solr/rest/TestRestManager.java

@@ -16,6 +16,7 @@ package org.apache.solr.rest;
  * limitations under the License.
  */
 
+import java.io.File;
 import java.util.Arrays;
 import java.util.Locale;
 import java.util.Set;
@@ -222,4 +223,29 @@ public class TestRestManager extends SolrRestletTestBase {
     // make sure it's really gone
     assertJQ("/config/managed", "/managedResources==[]");
   }
+  
+  @Test
+  public void testReloadFromPersistentStorage() throws Exception {
+    SolrResourceLoader loader = new SolrResourceLoader("./");
+    File unitTestStorageDir = createTempDir("testRestManager");
+    assertTrue(unitTestStorageDir.getAbsolutePath()+" is not a directory!", 
+        unitTestStorageDir.isDirectory());    
+    assertTrue(unitTestStorageDir.canRead());
+    assertTrue(unitTestStorageDir.canWrite());
+
+    NamedList<String> ioInitArgs = new NamedList<>();
+    ioInitArgs.add(ManagedResourceStorage.STORAGE_DIR_INIT_ARG, 
+        unitTestStorageDir.getAbsolutePath());
+    
+    StorageIO storageIO = new ManagedResourceStorage.FileStorageIO();
+    storageIO.configure(loader, ioInitArgs);
+    
+    NamedList<String> initArgs = new NamedList<>();
+    RestManager restManager = new RestManager();
+    restManager.init(loader, initArgs, storageIO);
+    
+    // verifies a RestManager can be reloaded from a previous RestManager's data
+    RestManager restManager2 = new RestManager();
+    restManager2.init(loader, initArgs, storageIO);    
+  }
 }