[Zeppelin-1561] Improve sync for multiuser environment

[khalidhuseynov]

What is this PR for?

apply multi-tenancy for storage sync mechanism

What type of PR is it?

Bug Fix | Improvement

Todos

• - broadcast on sync
• - set permissions for pulled notes
• - add test

What is the Jira issue?

ZEPPELIN-1561

How should this be tested?

Outline the steps to test the PR here.

Screenshots (if appropriate)

green CI

Questions:

• Does the licenses files need update? no
• Is there breaking changes for older versions? no
• Does this needs documentation? no

[anthonycorbacho]

Thank you for handeling this case, i added a comment please let me know what your think about it

[anthonycorbacho]

How about creating in AuthenticationInfo a new method called isAnonymous(AuthenticationInfo subject) and move the check inside?

something like

// In AuthenticationInfo.java
 private static final String ANONYMOUS = "anonymous";

 public static boolean isAnonymous(AuthenticationInfo subject) {
   if (subject != null ) {
     LOG.info("Subject is null assuming anonymous");
     return true;
   }
   return ANONYMOUS.equals(subject.getUser(); 
 }

//for the fun
public boolean isAnonymous() {
  return ANONYMOUS.equals(user);
}

//then here you can do something like this in makePrivate

if (AuthenticationInfo.isAnonymous(subject)) {
  // Add maybe meaningful log :)
  return;
}
NotebookAuthorization notebookAuthorization = NotebookAuthorization.getInstance();
Set<String> users = notebookAuthorization.getOwners(noteId);
users.add(subject.getUser());
notebookAuthorization.setOwners(noteId, users);
users = notebookAuthorization.getReaders(noteId);
users.add(subject.getUser());
notebookAuthorization.setReaders(noteId, users);
users = notebookAuthorization.getWriters(noteId);
users.add(subject.getUser());
notebookAuthorization.setWriters(noteId, users);

[khalidhuseynov]

makes sense, can be reused to make anonymous consistent in usage over codebase. I'll change now

[khalidhuseynov]

@anthonycorbacho addressed in d0a8c3c with some minor changes

[ghost]

I don't see syncOnStart method called anywhere. So, i believe you commented the above line by mistake.

[khalidhuseynov]

@rajarajan-g thanks for pointing to it. actually initial purpose was to delete it, but it was extracted into sub-function and commented out. addressed by deleting it in 1c11c2d

[Leemoonsoo]

Could you elaborate why 'makePrivate()' routine, which is being used inside of sync() adds current user as a owner, reader, writer?

[khalidhuseynov]

right, it sets it when pulling notes from secondary storage to your main one

[Leemoonsoo]

Can you explain why it sets when pulling notes from secondary storage to the main one?

For example, what happen if a note permission has a user as a reader, but not as a writer and owner?

[khalidhuseynov]

if it's pulling, that means there was no such a note in main storage and it's pulling new note from secondary storage. Thus, that new note not supposed to have previous owners, readers, writers. So generally it's same as creating note and only owner should be set. But I also set readers and writers in order other users don't see that new note (make it private).

[khalidhuseynov]

@Leemoonsoo your comment makes sense if there's already existing acl for the note. addressed it with test under e41b9b9

[khalidhuseynov]

CI is green, ready for review

[anthonycorbacho]

LGTM merging if no more discussion