[Zeppelin 537] Improve behavior of reloading notebooks from storage

docs/install/install.md

@@ -176,12 +176,6 @@ Configuration can be done by both environment variable(conf/zeppelin-env.sh) and
     <td>org.apache.zeppelin.notebook.repo.VFSNotebookRepo</td>
     <td>Comma separated list of notebook storage</td>
   </tr>
-  <tr>
-    <td>ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE</td>
-    <td>zeppelin.notebook.reloadAllNotesFromStorage</td>
-    <td>false</td>
-    <td>Notebook list and contents will be always loaded from repository if set true. If set false, modified notebooks or new notebooks added on file system level won't be reflected on Zeppelin till user restarts Zeppelin.</td>
-  </tr>
   <tr>
     <td>ZEPPELIN_INTERPRETERS</td>
     <td>zeppelin.interpreters</td>

zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java

@@ -131,7 +131,7 @@ public Response bind(@PathParam("noteId") String noteId) {
   @GET
   @Path("/")
   public Response getNotebookList() throws IOException {
-    List<Map<String, String>> notesInfo = notebookServer.generateNotebooksInfo();
+    List<Map<String, String>> notesInfo = notebookServer.generateNotebooksInfo(false);
     return new JsonResponse<>(Status.OK, "", notesInfo ).build();
   }
 

zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java

@@ -92,6 +92,7 @@ public static enum OP {
                      // @param completions list of string
 
     LIST_NOTES, // [c-s] ask list of note
+    RELOAD_NOTES_FROM_REPO, // [c-s] reload notes from repo
 
     NOTES_INFO, // [s-c] list of note infos
                 // @param notes serialized List<NoteInfo> object

zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java

@@ -101,6 +101,9 @@ public void onMessage(NotebookSocket conn, String msg) {
           case LIST_NOTES:
             broadcastNoteList();
             break;
+          case RELOAD_NOTES_FROM_REPO:
+            broadcastReloadedNoteList();
+            break;
           case GET_HOME_NOTE:
             sendHomeNote(conn, notebook);
             break;
@@ -291,14 +294,22 @@ private void broadcastAll(Message m) {
     }
   }
 
-  public List<Map<String, String>> generateNotebooksInfo (){
+  public List<Map<String, String>> generateNotebooksInfo(boolean needsReload) {
     Notebook notebook = notebook();
 
     ZeppelinConfiguration conf = notebook.getConf();
     String homescreenNotebookId = conf.getString(ConfVars.ZEPPELIN_NOTEBOOK_HOMESCREEN);
     boolean hideHomeScreenNotebookFromList = conf
             .getBoolean(ConfVars.ZEPPELIN_NOTEBOOK_HOMESCREEN_HIDE);
 
+    if (needsReload) {
+      try {
+        notebook.reloadAllNotes();
+      } catch (IOException e) {
+        LOG.error("Fail to reload notes from repository");
+      }
+    }
+
     List<Note> notes = notebook.getAllNotes();
     List<Map<String, String>> notesInfo = new LinkedList<>();
     for (Note note : notes) {
@@ -321,8 +332,12 @@ public void broadcastNote(Note note) {
   }
 
   public void broadcastNoteList() {
+    List<Map<String, String>> notesInfo = generateNotebooksInfo(false);
+    broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo));
+  }
 
-    List<Map<String, String>> notesInfo = generateNotebooksInfo();
+  public void broadcastReloadedNoteList() {
+    List<Map<String, String>> notesInfo = generateNotebooksInfo(true);
     broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo));
   }
 

zeppelin-web/src/app/home/home.controller.js

@@ -22,6 +22,8 @@ angular.module('zeppelinWebApp').controller('HomeCtrl', function($scope, noteboo
 
   vm.notebookHome = false;
   vm.staticHome = false;
+
+  $scope.isReloading = false;
 
   var initHome = function() {
     websocketMsgSrv.getHomeNotebook();
@@ -46,4 +48,13 @@ angular.module('zeppelinWebApp').controller('HomeCtrl', function($scope, noteboo
       vm.notebookHome = false;
     }
   });
+
+  $scope.$on('setNoteMenu', function(event, notes) {
+    $scope.isReloadingNotes = false;
+  });
+
+  $scope.reloadNotebookList = function() {
+    websocketMsgSrv.reloadAllNotesFromRepo();
+    $scope.isReloadingNotes = true;
+  };
 });

zeppelin-web/src/app/home/home.html

@@ -26,7 +26,12 @@ <h1 class="box-heading" id="welcome">
 
       <div class="row">
         <div class="col-md-4">
-          <h4>Notebook</h4>
+          <h4>Notebook 
+            <i ng-class="isReloadingNotes ? 'fa fa-refresh fa-spin' : 'fa fa-refresh'"
+              ng-style="!isReloadingNotes && {'cursor': 'pointer'}" style="font-size: 13px;"
+              ng-click="reloadNotebookList();">
+            </i>
+          </h4>
 
           <div>
             <h5><a href="" data-toggle="modal" data-target="#noteImportModal" style="text-decoration: none;">

zeppelin-web/src/components/websocketEvents/websocketMsg.service.js

@@ -32,10 +32,15 @@ angular.module('zeppelinWebApp').service('websocketMsgSrv', function($rootScope,
     cloneNotebook: function(noteIdToClone, newNoteName ) {
       websocketEvents.sendNewEvent({op: 'CLONE_NOTE', data: {id: noteIdToClone, name: newNoteName}});
     },
+
     getNotebookList: function() {
       websocketEvents.sendNewEvent({op: 'LIST_NOTES'});
     },
 
+    reloadAllNotesFromRepo: function() {
+      websocketEvents.sendNewEvent({op: 'RELOAD_NOTES_FROM_REPO'});
+    },
+
     getNotebook: function(noteId) {
       websocketEvents.sendNewEvent({op: 'GET_NOTE', data: {id: noteId}});
     },

zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java

@@ -426,10 +426,6 @@ public static enum ConfVars {
     ZEPPELIN_NOTEBOOK_S3_BUCKET("zeppelin.notebook.s3.bucket", "zeppelin"),
     ZEPPELIN_NOTEBOOK_S3_USER("zeppelin.notebook.s3.user", "user"),
     ZEPPELIN_NOTEBOOK_STORAGE("zeppelin.notebook.storage", VFSNotebookRepo.class.getName()),
-    // Notebook list and contents will be always loaded from repository if set true.
-    // If set false, modified notebooks or new notebooks added on file system level
-    // won't be reflected on Zeppelin till user restarts Zeppelin.
-    ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE("zeppelin.notebook.reloadAllNotesFromStorage", false),
     ZEPPELIN_INTERPRETER_REMOTE_RUNNER("zeppelin.interpreter.remoterunner", "bin/interpreter.sh"),
     // Decide when new note is created, interpreter settings will be binded automatically or not.
     ZEPPELIN_NOTEBOOK_AUTO_INTERPRETER_BINDING("zeppelin.notebook.autoInterpreterBinding", true),

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java

@@ -38,6 +38,7 @@
 import org.apache.zeppelin.interpreter.InterpreterSetting;
 import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry;
 import org.apache.zeppelin.notebook.repo.NotebookRepo;
+import org.apache.zeppelin.notebook.repo.NotebookRepoSync;
 import org.apache.zeppelin.scheduler.SchedulerFactory;
 import org.apache.zeppelin.search.SearchService;
 import org.quartz.CronScheduleBuilder;
@@ -330,10 +331,18 @@ private void loadAllNotes() throws IOException {
    * @return
    * @throws IOException
    */
-  private void reloadAllNotes() throws IOException {
+  public void reloadAllNotes() throws IOException {
     synchronized (notes) {
       notes.clear();
     }
+
+    if (notebookRepo instanceof NotebookRepoSync) {
+      NotebookRepoSync mainRepo = (NotebookRepoSync) notebookRepo;
+      if (mainRepo.getRepoCount() > 1) {
+        mainRepo.sync();
+      }
+    }
+
     List<NoteInfo> noteInfos = notebookRepo.list();
     for (NoteInfo info : noteInfos) {
       loadNoteFromRepo(info.getId());
@@ -366,13 +375,6 @@ public Date getLastUpdate() {
   }
 
   public List<Note> getAllNotes() {
-    if (conf.getBoolean(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE)) {
-      try {
-        reloadAllNotes();
-      } catch (IOException e) {
-        logger.error("Cannot reload notes from storage", e);
-      }
-    }
     synchronized (notes) {
       List<Note> noteList = new ArrayList<Note>(notes.values());
       Collections.sort(noteList, new Comparator<Note>() {

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java

@@ -41,7 +41,6 @@ public class NotebookRepoSync implements NotebookRepo {
   private static final int maxRepoNum = 2;
   private static final String pushKey = "pushNoteIDs";
   private static final String pullKey = "pullNoteIDs";
-  private static ZeppelinConfiguration config;
 
   private List<NotebookRepo> repos = new ArrayList<NotebookRepo>();
 
@@ -51,8 +50,6 @@ public class NotebookRepoSync implements NotebookRepo {
    * @throws - Exception
    */
   public NotebookRepoSync(ZeppelinConfiguration conf) throws Exception {
-    config = conf;
-
     String allStorageClassNames = conf.getString(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE).trim();
     if (allStorageClassNames.isEmpty()) {
       throw new IOException("Empty ZEPPELIN_NOTEBOOK_STORAGE conf parameter");
@@ -80,9 +77,6 @@ public NotebookRepoSync(ZeppelinConfiguration conf) throws Exception {
    */
   @Override
   public List<NoteInfo> list() throws IOException {
-    if (config.getBoolean(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE) && getRepoCount() > 1) {
-      sync(0, 1);
-    }
     return getRepo(0).list();
   }
 
@@ -182,7 +176,7 @@ private void pushNotes(List<String> ids, NotebookRepo localRepo,
     }
   }
 
-  int getRepoCount() {
+  public int getRepoCount() {
     return repos.size();
   }
 

zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java

@@ -47,7 +47,6 @@
 import org.apache.zeppelin.scheduler.JobListener;
 import org.apache.zeppelin.scheduler.SchedulerFactory;
 import org.apache.zeppelin.search.SearchService;
-import org.apache.zeppelin.search.LuceneSearch;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -122,42 +121,39 @@ public void testSelectingReplImplementation() throws IOException {
   }
 
   @Test
-  public void testGetAllNotes() throws IOException {
-    // get all notes after copy the {notebookId}/note.json into notebookDir
+  public void testReloadAllNotes() throws IOException {
     File srcDir = new File("src/test/resources/2A94M5J1Z");
     File destDir = new File(notebookDir.getAbsolutePath() + "/2A94M5J1Z");
 
+    // copy the notebook
     try {
       FileUtils.copyDirectory(srcDir, destDir);
     } catch (IOException e) {
       e.printStackTrace();
     }
 
-    Note copiedNote = notebookRepo.get("2A94M5J1Z");
-
-    // when ZEPPELIN_NOTEBOOK_GET_FROM_REPO set to be false
-    System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE.getVarName(), "false");
+    // doesn't have copied notebook in memory before reloading
     List<Note> notes = notebook.getAllNotes();
     assertEquals(notes.size(), 0);
 
-    // when ZEPPELIN_NOTEBOOK_GET_FROM_REPO set to be true
-    System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE.getVarName(), "true");
+    // load copied notebook on memory when reloadAllNotes() is called
+    Note copiedNote = notebookRepo.get("2A94M5J1Z");
+    notebook.reloadAllNotes();
     notes = notebook.getAllNotes();
     assertEquals(notes.size(), 1);
     assertEquals(notes.get(0).id(), copiedNote.id());
     assertEquals(notes.get(0).getName(), copiedNote.getName());
     assertEquals(notes.get(0).getParagraphs(), copiedNote.getParagraphs());
 
-    // get all notes after remove the {notebookId}/note.json from notebookDir
-    // when ZEPPELIN_NOTEBOOK_GET_FROM_REPO set to be false
-    System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE.getVarName(), "false");
     // delete the notebook
     FileUtils.deleteDirectory(destDir);
+
+    // keep notebook in memory before reloading
     notes = notebook.getAllNotes();
     assertEquals(notes.size(), 1);
 
-    // when ZEPPELIN_NOTEBOOK_GET_FROM_REPO set to be true
-    System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE.getVarName(), "true");
+    // delete notebook from notebook list when reloadAllNotes() is called
+    notebook.reloadAllNotes();
     notes = notebook.getAllNotes();
     assertEquals(notes.size(), 0);
   }

zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java

@@ -184,41 +184,32 @@ public void testSyncUpdateMain() throws IOException {
     assertEquals(p1.getId(), notebookRepoSync.get(1,
         notebookRepoSync.list(1).get(0).getId()).getLastParagraph().getId());
   }
-  
+
   @Test
-  public void testSyncOnList() throws IOException {
-
-	/* check that both storage repos are empty */
-	assertTrue(notebookRepoSync.getRepoCount() > 1);
-	assertEquals(0, notebookRepoSync.list(0).size());
-	assertEquals(0, notebookRepoSync.list(1).size());
-
-	File srcDir = new File("src/test/resources/2A94M5J1Z");
-	File destDir = new File(secNotebookDir + "/2A94M5J1Z");
-
-	/* copy manually new notebook into secondary storage repo and check repos */
+  public void testSyncOnReloadedList() throws IOException {
+    /* check that both storage repos are empty */
+    assertTrue(notebookRepoSync.getRepoCount() > 1);
+    assertEquals(0, notebookRepoSync.list(0).size());
+    assertEquals(0, notebookRepoSync.list(1).size());
+
+    File srcDir = new File("src/test/resources/2A94M5J1Z");
+    File destDir = new File(secNotebookDir + "/2A94M5J1Z");
+
+    /* copy manually new notebook into secondary storage repo and check repos */
     try {
-        FileUtils.copyDirectory(srcDir, destDir);
-      } catch (IOException e) {
-        e.printStackTrace();
-      }
+      FileUtils.copyDirectory(srcDir, destDir);
+    } catch (IOException e) {
+      e.printStackTrace();
+    }
     assertEquals(0, notebookRepoSync.list(0).size());
     assertEquals(1, notebookRepoSync.list(1).size());
-
-    /* Although new notebook is added to secondary storage it's not displayed 
-     * on list() with ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE set to false
-     */
-    System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE.getVarName(), "false");
-    assertEquals(0, notebookRepoSync.list().size());
-
-    /* notebook is synced after ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE variable is set to true */
-    System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_RELOAD_FROM_STORAGE.getVarName(), "true");
-    assertEquals(1, notebookRepoSync.list().size());
-
+
+    // After reloading notebooks repos should be synchronized
+    notebookSync.reloadAllNotes();
     assertEquals(1, notebookRepoSync.list(0).size());
-	assertEquals(1, notebookRepoSync.list(1).size());
+    assertEquals(1, notebookRepoSync.list(1).size());
   }
-  
+
   static void delete(File file){
     if(file.isFile()) file.delete();
       else if(file.isDirectory()){
@@ -230,8 +221,8 @@ else if(file.isDirectory()){
         }
         file.delete();
       }
-
   }
+
   @Override
   public JobListener getParagraphJobListener(Note note) {
     return new JobListener(){