address comment + test

khalidhuseynov · khalidhuseynov · commit bd1a44afdd45 · 2016-10-23T21:41:41.000+09:00
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
@@ -166,6 +166,10 @@ public void remove(String noteId, AuthenticationInfo subject) throws IOException
     /* TODO(khalid): handle case when removing from secondary storage fails */
   }
 
+  void remove(int repoIndex, String noteId, AuthenticationInfo subject) throws IOException {
+    getRepo(repoIndex).remove(noteId, subject);
+  }
+
   /**
    * Copies new/updated notes from source to destination storage
    *
@@ -228,7 +232,7 @@ private void pushNotes(AuthenticationInfo subject, List<String> ids, NotebookRep
     for (String id : ids) {
       try {
         remoteRepo.save(localRepo.get(id, subject), subject);
-        if (setPermissions) {
+        if (setPermissions && emptyNoteAcl(id)) {
           makePrivate(id, subject);
         }
       } catch (IOException e) {
@@ -237,6 +241,13 @@ private void pushNotes(AuthenticationInfo subject, List<String> ids, NotebookRep
     }
   }
 
+  private boolean emptyNoteAcl(String noteId) {
+    NotebookAuthorization notebookAuthorization = NotebookAuthorization.getInstance();
+    return notebookAuthorization.getOwners(noteId).isEmpty()
+        && notebookAuthorization.getReaders(noteId).isEmpty()
+        && notebookAuthorization.getWriters(noteId).isEmpty();
+  }
+
   private void makePrivate(String noteId, AuthenticationInfo subject) {
     if (AuthenticationInfo.isAnonymous(subject)) {
       LOG.info("User is anonymous, permissions are not set for pulled notes");
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java
@@ -24,7 +24,9 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.zeppelin.conf.ZeppelinConfiguration;
@@ -314,6 +316,72 @@ public void testCheckpointOneStorage() throws IOException, SchedulerException {
     notebookRepoSync.remove(note.getId(), anonymous);
   }
   
+  @Test
+  public void testSyncWithAcl() throws IOException {
+    /* scenario 1 - note exists with acl on main storage */
+    AuthenticationInfo user1 = new AuthenticationInfo("user1");
+    Note note = notebookSync.createNote(user1);
+    assertEquals(0, note.getParagraphs().size());
+    
+    // saved on both storages
+    assertEquals(1, notebookRepoSync.list(0, null).size());
+    assertEquals(1, notebookRepoSync.list(1, null).size());
+    
+    /* check that user1 is the only owner */
+    NotebookAuthorization authInfo = NotebookAuthorization.getInstance();
+    Set<String> entity = new HashSet<String>();
+    entity.add(user1.getUser());
+    assertEquals(true, authInfo.isOwner(note.getId(), entity));
+    assertEquals(1, authInfo.getOwners(note.getId()).size());
+    assertEquals(0, authInfo.getReaders(note.getId()).size());
+    assertEquals(0, authInfo.getWriters(note.getId()).size());
+    
+    /* update note and save on secondary storage */
+    Paragraph p1 = note.addParagraph();
+    p1.setText("hello world");
+    assertEquals(1, note.getParagraphs().size());
+    notebookRepoSync.save(1, note, null);
+    
+    /* check paragraph isn't saved into first storage */
+    assertEquals(0, notebookRepoSync.get(0,
+        notebookRepoSync.list(0, null).get(0).getId(), null).getParagraphs().size());
+    /* check paragraph is saved into second storage */
+    assertEquals(1, notebookRepoSync.get(1,
+        notebookRepoSync.list(1, null).get(0).getId(), null).getParagraphs().size());
+    
+    /* now sync by user1 */
+    notebookRepoSync.sync(user1);
+    
+    /* check that note updated and acl are same on main storage*/
+    assertEquals(1, notebookRepoSync.get(0,
+        notebookRepoSync.list(0, null).get(0).getId(), null).getParagraphs().size());
+    assertEquals(true, authInfo.isOwner(note.getId(), entity));
+    assertEquals(1, authInfo.getOwners(note.getId()).size());
+    assertEquals(0, authInfo.getReaders(note.getId()).size());
+    assertEquals(0, authInfo.getWriters(note.getId()).size());
+    
+    /* scenario 2 - note doesn't exist on main storage */
+    /* remove from main storage */
+    notebookRepoSync.remove(0, note.getId(), user1);
+    assertEquals(0, notebookRepoSync.list(0, null).size());
+    assertEquals(1, notebookRepoSync.list(1, null).size());
+    authInfo.removeNote(note.getId());
+    assertEquals(0, authInfo.getOwners(note.getId()).size());
+    assertEquals(0, authInfo.getReaders(note.getId()).size());
+    assertEquals(0, authInfo.getWriters(note.getId()).size());
+    
+    /* now sync - should bring note from secondary storage with added acl */
+    notebookRepoSync.sync(user1);
+    assertEquals(1, notebookRepoSync.list(0, null).size());
+    assertEquals(1, notebookRepoSync.list(1, null).size());
+    assertEquals(1, authInfo.getOwners(note.getId()).size());
+    assertEquals(1, authInfo.getReaders(note.getId()).size());
+    assertEquals(1, authInfo.getWriters(note.getId()).size());
+    assertEquals(true, authInfo.isOwner(note.getId(), entity));
+    assertEquals(true, authInfo.isReader(note.getId(), entity));
+    assertEquals(true, authInfo.isWriter(note.getId(), entity));
+  }
+
   static void delete(File file){
     if(file.isFile()) file.delete();
       else if(file.isDirectory()){
