Skip to content

Commit ac76546

Browse files
committed
[ZEPPELIN-6556] Personalized mode leaks a non-owner's paragraph edits into the shared master paragraph
1 parent c11fe27 commit ac76546

2 files changed

Lines changed: 68 additions & 31 deletions

File tree

zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,9 @@ public boolean runParagraph(Note note,
452452
callback.onFailure(new IOException("paragraph is disabled."), context);
453453
return false;
454454
}
455+
if (note.isPersonalizedMode()) {
456+
p = p.getUserParagraph(context.getAutheInfo().getUser());
457+
}
455458
p.setText(text);
456459
p.setTitle(title);
457460
p.setAuthenticationInfo(context.getAutheInfo());
@@ -462,19 +465,6 @@ public boolean runParagraph(Note note,
462465
p.mergeConfig(config);
463466
}
464467

465-
if (note.isPersonalizedMode()) {
466-
p = p.getUserParagraph(context.getAutheInfo().getUser());
467-
p.setText(text);
468-
p.setTitle(title);
469-
p.setAuthenticationInfo(context.getAutheInfo());
470-
if (params != null && !params.isEmpty()) {
471-
p.settings.setParams(params);
472-
}
473-
if (config != null && !config.isEmpty()) {
474-
p.mergeConfig(config);
475-
}
476-
}
477-
478468
try {
479469
notebook.saveNote(note, context.getAutheInfo());
480470
note.run(p.getId(), sessionId, blocking, context.getAutheInfo().getUser());
@@ -761,17 +751,13 @@ public void updateParagraph(String noteId,
761751
callback.onFailure(new ParagraphNotFoundException(paragraphId), context);
762752
return null;
763753
}
754+
if (note.isPersonalizedMode()) {
755+
p = p.getUserParagraph(context.getAutheInfo().getUser());
756+
}
764757
p.settings.setParams(params);
765758
p.mergeConfig(config);
766759
p.setTitle(title);
767760
p.setText(text);
768-
if (note.isPersonalizedMode()) {
769-
p = p.getUserParagraph(context.getAutheInfo().getUser());
770-
p.settings.setParams(params);
771-
p.mergeConfig(config);
772-
p.setTitle(title);
773-
p.setText(text);
774-
}
775761
notebook.saveNote(note, context.getAutheInfo());
776762
callback.onSuccess(p, context);
777763
return null;
@@ -1393,23 +1379,17 @@ private Paragraph setParagraphUsingMessage(Note note, Message fromMessage, Strin
13931379
String text, String title, Map<String, Object> params,
13941380
Map<String, Object> config) {
13951381
Paragraph p = note.getParagraph(paragraphId);
1396-
p.setText(text);
1397-
p.setTitle(title);
13981382
AuthenticationInfo subject =
13991383
new AuthenticationInfo(fromMessage.principal, fromMessage.roles, fromMessage.ticket);
1384+
if (note.isPersonalizedMode()) {
1385+
p = p.getUserParagraph(subject.getUser());
1386+
}
1387+
p.setText(text);
1388+
p.setTitle(title);
14001389
p.setAuthenticationInfo(subject);
14011390
p.settings.setParams(params);
14021391
p.setConfig(config);
14031392

1404-
if (note.isPersonalizedMode()) {
1405-
p = note.getParagraph(paragraphId);
1406-
p.setText(text);
1407-
p.setTitle(title);
1408-
p.setAuthenticationInfo(subject);
1409-
p.settings.setParams(params);
1410-
p.setConfig(config);
1411-
}
1412-
14131393
return p;
14141394
}
14151395

zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,63 @@ void testParagraphOperations() throws IOException {
588588
verify(callback).onSuccess(p, context);
589589
}
590590

591+
@Test
592+
void testRunParagraphInPersonalizedModeDoesNotPolluteMasterParagraph() throws IOException {
593+
String note1Id = notebookService.createNote("/note_personalized", "test", true, context, callback);
594+
Map<String, Object> masterParams = new HashMap<>();
595+
masterParams.put("name", "master");
596+
String paragraphId = notebook.processNote(note1Id,
597+
note1 -> {
598+
note1.setPersonalizedMode(true);
599+
Paragraph p = note1.getParagraph(0);
600+
p.setText("1+1");
601+
p.settings.setParams(masterParams);
602+
return p.getId();
603+
});
604+
605+
ServiceContext user1Context =
606+
new ServiceContext(new AuthenticationInfo("user1"), new HashSet<>());
607+
Map<String, Object> user1Params = new HashMap<>();
608+
user1Params.put("name", "user1");
609+
610+
reset(callback);
611+
boolean runStatus = notebook.processNote(note1Id,
612+
note1 -> {
613+
return notebookService.runParagraph(note1, paragraphId, "user1_title", "1+1",
614+
user1Params, new HashMap<>(), null, false, true, user1Context, callback);
615+
});
616+
assertTrue(runStatus);
617+
618+
notebook.processNote(note1Id,
619+
note1 -> {
620+
Paragraph master = note1.getParagraph(paragraphId);
621+
assertEquals(masterParams, master.settings.getParams());
622+
assertNull(master.getTitle());
623+
Paragraph user1Paragraph = master.getUserParagraph("user1");
624+
assertEquals(user1Params, user1Paragraph.settings.getParams());
625+
assertEquals("user1_title", user1Paragraph.getTitle());
626+
return null;
627+
});
628+
629+
// updateParagraph must not pollute the master paragraph either
630+
reset(callback);
631+
Map<String, Object> user1UpdatedParams = new HashMap<>();
632+
user1UpdatedParams.put("name", "user1_updated");
633+
notebookService.updateParagraph(note1Id, paragraphId, "user1_updated_title", "1+1",
634+
user1UpdatedParams, new HashMap<>(), user1Context, callback);
635+
636+
notebook.processNote(note1Id,
637+
note1 -> {
638+
Paragraph master = note1.getParagraph(paragraphId);
639+
assertEquals(masterParams, master.settings.getParams());
640+
assertNull(master.getTitle());
641+
Paragraph user1Paragraph = master.getUserParagraph("user1");
642+
assertEquals(user1UpdatedParams, user1Paragraph.settings.getParams());
643+
assertEquals("user1_updated_title", user1Paragraph.getTitle());
644+
return null;
645+
});
646+
}
647+
591648
@Test
592649
void testNormalizeNotePath() throws IOException {
593650
assertEquals("/Untitled Note", notebookService.normalizeNotePath(" "));

0 commit comments

Comments
 (0)