Merge pull request #6949 from jasongrout/comminfo

Fix comm_info_request content to conform to spec in a backwards-compatible way
jupyterlab · Aug 6, 2019 · f2d664b · f2d664b
2 parents 5b4c48f + d752554
commit f2d664b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 2 deletions.
diff --git a/packages/services/src/kernel/messages.ts b/packages/services/src/kernel/messages.ts
@@ -143,6 +143,18 @@ export namespace KernelMessage {
   ): T;
 
   export function createMessage<T extends Message>(options: IOptions<T>): T {
+    // Backwards compatibility workaround for services 4.0 defining the wrong
+    // comm_info_request content. This should be removed with the deprecated
+    // `target` content option in services 5.0. See
+    // https://github.com/jupyterlab/jupyterlab/issues/6947
+    if (options.msgType === 'comm_info_request') {
+      const content = options.content as ICommInfoRequestMsg['content'];
+      if (content.target_name === undefined) {
+        content.target_name = content.target;
+      }
+      delete content.target;
+    }
+
     return {
       buffers: options.buffers || [],
       channel: options.channel,
@@ -1076,10 +1088,22 @@ export namespace KernelMessage {
    *
    * **See also:** [[ICommInfoReplyMsg]], [[IKernel.commInfo]]
    */
-
   export interface ICommInfoRequestMsg
     extends IShellMessage<'comm_info_request'> {
     content: {
+      /**
+       * The comm target name to filter returned comms
+       */
+      target_name?: string;
+
+      /**
+       * Filter for returned comms
+       *
+       * @deprecated - this is a non-standard field. Use target_name instead
+       *
+       * #### Notes
+       * See https://github.com/jupyterlab/jupyterlab/issues/6947
+       */
       target?: string;
     };
   }
@@ -1091,7 +1115,6 @@ export namespace KernelMessage {
    *
    * **See also:** [[ICommInfoRequest]], [[IKernel.commInfo]]
    */
-
   export interface ICommInfoReply extends IReplyOkContent {
     /**
      * Mapping of comm ids to target names.

diff --git a/tests/test-services/src/kernel/messages.spec.ts b/tests/test-services/src/kernel/messages.spec.ts
@@ -168,4 +168,33 @@ describe('kernel/messages', () => {
       expect(KernelMessage.isInputRequestMsg(msg2)).to.equal(false);
     });
   });
+
+  describe('KernelMessage.createMessage()', () => {
+    // Tests deprecated option workaround. Should be deleted in services 5.0.
+    // See https://github.com/jupyterlab/jupyterlab/pull/6949
+    it('contains a backwards-compatibility workaround for services 4.0 for a deprecated comm_info_request content', () => {
+      let commRequest = KernelMessage.createMessage({
+        msgType: 'comm_info_request',
+        channel: 'shell',
+        session: 'baz',
+        content: {
+          target: 'example'
+        }
+      });
+      expect(commRequest.content.target_name).to.equal('example');
+      expect(commRequest.content.target).to.be.undefined;
+
+      commRequest = KernelMessage.createMessage({
+        msgType: 'comm_info_request',
+        channel: 'shell',
+        session: 'baz',
+        content: {
+          target_name: 'real_target',
+          target: 'example'
+        }
+      });
+      expect(commRequest.content.target_name).to.equal('real_target');
+      expect(commRequest.content.target).to.be.undefined;
+    });
+  });
 });