diff --git a/kernel/base/src/main/java/com/twosigma/beakerx/kernel/comm/Comm.java b/kernel/base/src/main/java/com/twosigma/beakerx/kernel/comm/Comm.java index 3df7c0f339..7d9b1acd11 100644 --- a/kernel/base/src/main/java/com/twosigma/beakerx/kernel/comm/Comm.java +++ b/kernel/base/src/main/java/com/twosigma/beakerx/kernel/comm/Comm.java @@ -23,8 +23,6 @@ import com.twosigma.beakerx.kernel.msg.JupyterMessages; import com.twosigma.beakerx.message.Header; import com.twosigma.beakerx.message.Message; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.Serializable; import java.util.ArrayList; @@ -36,11 +34,8 @@ import static com.twosigma.beakerx.kernel.msg.JupyterMessages.COMM_MSG; import static com.twosigma.beakerx.kernel.msg.JupyterMessages.COMM_OPEN; - public class Comm { - private static final Logger logger = LoggerFactory.getLogger(Comm.class); - public static final String METHOD = "method"; public static final String UPDATE = "update"; public static final String STATE = "state"; @@ -62,7 +57,6 @@ public class Comm { private KernelFunctionality kernel; private List> msgCallbackList = new ArrayList<>(); private List> closeCallbackList = new ArrayList<>(); - private Message parentMessage; public Comm(String commId, String targetName) { super(); @@ -134,13 +128,12 @@ public void clearCloseCallbackList() { } public void open() { - this.parentMessage = InternalVariable.getParentHeader(); - doOpen(parentMessage); + doOpen(getParentMessage()); } public void open(Message parentMessage) { - this.parentMessage = parentMessage; - doOpen(parentMessage); + getParentMessageStrategy = () -> parentMessage; + open(); } private void doOpen(Message parentMessage) { @@ -167,7 +160,7 @@ private void doOpen(Message parentMessage) { } public void close() { - Message parentMessage = getParentMessage();// can be null + Message parentMessage = getParentMessage(); if (this.getCloseCallbackList() != null && !this.getCloseCallbackList().isEmpty()) { for (Handler handler : getCloseCallbackList()) { @@ -225,9 +218,6 @@ public void sendUpdate(final String propertyName, final Object value) { this.send(); } - private Message getParentMessage() { - return this.parentMessage; - } public void handleMsg(Message parentMessage) { for (Handler handler : this.msgCallbackList) { @@ -247,4 +237,13 @@ public String toString() { return commId + "/" + targetName + "/" + (targetModule != null && !targetModule.isEmpty() ? targetModule : ""); } + public Message getParentMessage() { + return getParentMessageStrategy.getParentMessage(); + } + + private GetParentMessageStrategy getParentMessageStrategy = InternalVariable::getParentHeader; + + interface GetParentMessageStrategy { + Message getParentMessage(); + } } \ No newline at end of file diff --git a/kernel/base/src/main/java/com/twosigma/beakerx/widgets/Widget.java b/kernel/base/src/main/java/com/twosigma/beakerx/widgets/Widget.java index 1d5a0e08da..70de97eaac 100644 --- a/kernel/base/src/main/java/com/twosigma/beakerx/widgets/Widget.java +++ b/kernel/base/src/main/java/com/twosigma/beakerx/widgets/Widget.java @@ -21,7 +21,6 @@ import static com.twosigma.beakerx.kernel.msg.JupyterMessages.DISPLAY_DATA; import static com.twosigma.beakerx.widgets.CompiledCodeRunner.runCommEvent; -import com.twosigma.beakerx.evaluator.InternalVariable; import com.twosigma.beakerx.kernel.KernelManager; import com.twosigma.beakerx.kernel.comm.Comm; import com.twosigma.beakerx.kernel.comm.TargetNamesEnum; @@ -62,7 +61,9 @@ public Widget() { } protected void openComm() { - openComm(InternalVariable.getParentHeader()); + comm.setData(createContent()); + addValueChangeMsgCallback(); + comm.open(); } protected void openComm(Message parentMessage) { diff --git a/kernel/base/src/test/java/com/twosigma/beakerx/jupyter/comm/CommTest.java b/kernel/base/src/test/java/com/twosigma/beakerx/jupyter/comm/CommTest.java index 509e0e0a5b..cd5c89fdb3 100644 --- a/kernel/base/src/test/java/com/twosigma/beakerx/jupyter/comm/CommTest.java +++ b/kernel/base/src/test/java/com/twosigma/beakerx/jupyter/comm/CommTest.java @@ -17,10 +17,11 @@ package com.twosigma.beakerx.jupyter.comm; import com.twosigma.beakerx.KernelTest; +import com.twosigma.beakerx.evaluator.InternalVariable; +import com.twosigma.beakerx.jvm.object.SimpleEvaluationObject; import com.twosigma.beakerx.kernel.KernelManager; import com.twosigma.beakerx.kernel.msg.JupyterMessages; import com.twosigma.beakerx.kernel.comm.Comm; -import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; import com.twosigma.beakerx.message.Message; @@ -31,6 +32,8 @@ import java.util.HashMap; import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; + public class CommTest { private KernelTest kernel; @@ -48,13 +51,48 @@ public void tearDown() throws Exception { KernelManager.register(null); } + @Test + public void commCreatedWithParentMessageShouldAlwaysSendHeaderFromThisParentMessage() throws NoSuchAlgorithmException { + //given + submitCodeToExecution(); + //when + Message parentMessage = new Message(); + comm.open(parentMessage); + assertThat(kernel.getPublishedMessages().get(0).getParentHeader()).isEqualTo(parentMessage.getHeader()); + kernel.clearPublishedMessages(); + //then + comm.send(); + assertThat(kernel.getPublishedMessages().get(0).getParentHeader()).isEqualTo(parentMessage.getHeader()); + } + + @Test + public void commCreatedWithoutParentMessageShouldAlwaysSendHeaderFromMessageGivenFromInternalVariable() throws NoSuchAlgorithmException { + // code from first execution + Message message1 = submitCodeToExecution(); + comm.open(); + assertThat(kernel.getPublishedMessages().get(0).getParentHeader()).isEqualTo(message1.getHeader()); + kernel.clearPublishedMessages(); + // code from second execution + Message message2 = submitCodeToExecution(); + comm.send(); + assertThat(kernel.getPublishedMessages().get(0).getParentHeader()).isEqualTo(message2.getHeader()); + } + + private Message submitCodeToExecution() { + SimpleEvaluationObject value = new SimpleEvaluationObject("ok"); + Message jupyterMessage = new Message(); + value.setJupyterMessage(jupyterMessage); + InternalVariable.setValue(value); + return jupyterMessage; + } + @Test public void commOpen_shouldSendIOPubSocketMessage() throws NoSuchAlgorithmException { //when comm.open(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); - Assertions.assertThat(kernel.getPublishedMessages().get(0)).isNotNull(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages().get(0)).isNotNull(); } @Test @@ -62,7 +100,7 @@ public void commOpen_shouldAddCommToStorageMap() throws NoSuchAlgorithmException //when comm.open(); //then - Assertions.assertThat(kernel.isCommPresent(comm.getCommId())).isTrue(); + assertThat(kernel.isCommPresent(comm.getCommId())).isTrue(); } @Test @@ -70,10 +108,10 @@ public void commOpen_sentMessageHasTypeIsCommOpen() throws NoSuchAlgorithmExcept //when comm.open(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat(sendMessage.getHeader().getType()) - .isEqualTo(JupyterMessages.COMM_OPEN.getName()); + assertThat(sendMessage.getHeader().getType()) + .isEqualTo(JupyterMessages.COMM_OPEN.getName()); } @Test @@ -81,9 +119,9 @@ public void commOpen_sentMessageHasCommId() throws NoSuchAlgorithmException { //when comm.open(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((String) sendMessage.getContent().get(Comm.COMM_ID)).isNotEmpty(); + assertThat((String) sendMessage.getContent().get(Comm.COMM_ID)).isNotEmpty(); } @Test @@ -91,9 +129,9 @@ public void commOpen_sentMessageHasTargetName() throws NoSuchAlgorithmException //when comm.open(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((String) sendMessage.getContent().get(Comm.TARGET_NAME)).isNotEmpty(); + assertThat((String) sendMessage.getContent().get(Comm.TARGET_NAME)).isNotEmpty(); } @Test @@ -102,9 +140,9 @@ public void commOpen_sentMessageHasData() throws NoSuchAlgorithmException { //when comm.open(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((Map) sendMessage.getContent().get(Comm.DATA)).isNotEmpty(); + assertThat((Map) sendMessage.getContent().get(Comm.DATA)).isNotEmpty(); } @Test @@ -114,9 +152,9 @@ public void commOpen_sentMessageHasTargetModule() throws NoSuchAlgorithmExceptio //when comm.open(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((String) sendMessage.getContent().get(Comm.TARGET_MODULE)).isNotEmpty(); + assertThat((String) sendMessage.getContent().get(Comm.TARGET_MODULE)).isNotEmpty(); } @Test @@ -124,8 +162,8 @@ public void commClose_shouldSendIOPubSocketMessage() throws NoSuchAlgorithmExcep //when comm.close(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); - Assertions.assertThat(kernel.getPublishedMessages().get(0)).isNotNull(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages().get(0)).isNotNull(); } @Test @@ -133,7 +171,7 @@ public void commClose_shouldRemoveCommFromStorageMap() throws NoSuchAlgorithmExc //when comm.close(); //then - Assertions.assertThat(kernel.isCommPresent(comm.getCommId())).isFalse(); + assertThat(kernel.isCommPresent(comm.getCommId())).isFalse(); } @Test @@ -141,10 +179,10 @@ public void commClose_sentMessageHasTypeIsCommClose() throws NoSuchAlgorithmExce //when comm.close(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat(sendMessage.getHeader().getType()) - .isEqualTo(JupyterMessages.COMM_CLOSE.getName()); + assertThat(sendMessage.getHeader().getType()) + .isEqualTo(JupyterMessages.COMM_CLOSE.getName()); } @Test @@ -153,9 +191,9 @@ public void commClose_sentMessageHasEmptyData() throws NoSuchAlgorithmException //when comm.close(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((Map) sendMessage.getContent().get(Comm.DATA)).isEmpty(); + assertThat((Map) sendMessage.getContent().get(Comm.DATA)).isEmpty(); } @Test @@ -163,8 +201,8 @@ public void commSend_shouldSendIOPubSocketMessage() throws NoSuchAlgorithmExcept //when comm.send(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); - Assertions.assertThat(kernel.getPublishedMessages().get(0)).isNotNull(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages().get(0)).isNotNull(); } @Test @@ -172,10 +210,10 @@ public void commSend_sentMessageHasTypeIsCommClose() throws NoSuchAlgorithmExcep //when comm.send(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat(sendMessage.getHeader().getType()) - .isEqualTo(JupyterMessages.COMM_MSG.getName()); + assertThat(sendMessage.getHeader().getType()) + .isEqualTo(JupyterMessages.COMM_MSG.getName()); } @Test @@ -183,9 +221,9 @@ public void commSend_sentMessageHasCommId() throws NoSuchAlgorithmException { //when comm.send(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((String) sendMessage.getContent().get(Comm.COMM_ID)).isNotEmpty(); + assertThat((String) sendMessage.getContent().get(Comm.COMM_ID)).isNotEmpty(); } @Test @@ -194,9 +232,9 @@ public void commClose_sentMessageHasData() throws NoSuchAlgorithmException { //when comm.send(); //then - Assertions.assertThat(kernel.getPublishedMessages()).isNotEmpty(); + assertThat(kernel.getPublishedMessages()).isNotEmpty(); Message sendMessage = kernel.getPublishedMessages().get(0); - Assertions.assertThat((Map) sendMessage.getContent().get(Comm.DATA)).isNotEmpty(); + assertThat((Map) sendMessage.getContent().get(Comm.DATA)).isNotEmpty(); } private void initCommData(Comm comm) {