Simplify IPC code.

Author: RubenVerborgh

popup-app/components/IdpCallback.js

@@ -1,18 +1,17 @@
 import React, { Component } from 'react'
 
 import auth from '../../src'
-import { client } from '../../src/ipc'
+import { Client } from '../../src/ipc'
 import { postMessageStorage } from '../../src/storage'
 
 export default class IdpCallback extends Component {
   state = { loggedIn: false }
-
-  request = client(window.opener, this.props.appOrigin)
+  client = new Client(window.opener, this.props.appOrigin)
 
   postSession = async () => {
     const storage = postMessageStorage(window.opener, this.props.appOrigin)
     const session = await auth.currentSession(storage)
-    return this.request({ method: 'foundSession', args: [session] })
+    await this.client.request('foundSession', session)
   }
 
   constructor(props) {

popup-app/components/IdpSelect.js

@@ -1,7 +1,7 @@
 import React from 'react'
 
 import auth from '../../src'
-import { client } from '../../src/ipc'
+import { Client } from '../../src/ipc'
 import { postMessageStorage } from '../../src/storage'
 
 import './IdpSelect.css'
@@ -41,14 +41,8 @@ class IdpSelect extends React.Component {
       })
       return
     }
-    const request = client(window.opener, appOrigin)
-    let loginOptions = await timeout(
-      request({
-        method: 'getLoginOptions',
-        args: []
-      }),
-      2000
-    )
+    const client = new Client(window.opener, appOrigin)
+    let loginOptions = await timeout(client.request('getLoginOptions'), 2000)
     if (!loginOptions) {
       console.warn(
         'Cannot log in - have not yet received loginOptions from parent window'

popup-app/index.js

@@ -2,7 +2,7 @@
 import React from 'react'
 import ReactDOM from 'react-dom'
 
-import { client } from '../src/ipc'
+import { Client } from '../src/ipc'
 import { getData, updateStorage } from '../src/storage'
 
 import IdpCallback from './components/IdpCallback'
@@ -62,11 +62,8 @@ async function findAppOrigin() {
   if (appOrigin) {
     return appOrigin
   }
-  const request = client(window.opener, '*')
-  appOrigin = await request({
-    method: 'getAppOrigin',
-    args: []
-  })
+  const client = new Client(window.opener, '*')
+  appOrigin = await client.request('getAppOrigin')
   await storeAppOrigin(appOrigin)
   return appOrigin
 }

src/__test__/ipc.spec.js

@@ -2,20 +2,20 @@
 /* eslint-env jest */
 
 import { polyfillWindow, polyunfillWindow } from './spec-helpers'
-import { client, server } from '../ipc'
+import { Client, Server } from '../ipc'
 
 beforeEach(polyfillWindow)
 
 afterEach(polyunfillWindow)
 
-describe('client', () => {
+describe('Client', () => {
   it('takes a request, sets up a channel to the parent window, and promises a response from the parent', async () => {
     expect.assertions(5)
     const parent = window
     // window.open is not yet implemented in jsdom.  for the purposes of testing, this does just fine.
     const child = parent
     const origin = window.location.origin
-    const request = client(parent, origin)
+    const client = new Client(parent, origin)
     parent.addEventListener('message', function testServer(event) {
       const { data, origin } = event
       const isRequest =
@@ -42,7 +42,7 @@ describe('client', () => {
         parent.removeEventListener('message', testServer)
       }
     })
-    const response = await request({ method: 'foo', args: ['bar', 'baz'] })
+    const response = await client.request('foo', 'bar', 'baz')
     expect(response).toBe('the return value!')
   })
 
@@ -52,7 +52,7 @@ describe('client', () => {
     // window.open is not yet implemented in jsdom.  for the purposes of testing, this does just fine.
     const child = parent
     const origin = window.location.origin
-    const request = client(parent, origin)
+    const client = new Client(parent, origin)
     parent.addEventListener('message', function testServer(event) {
       const { data, origin } = event
       const isRequest =
@@ -88,61 +88,56 @@ describe('client', () => {
         parent.removeEventListener('message', testServer)
       }
     })
-    const response = await request({ method: 'foo', args: ['bar', 'baz'] })
+    const response = await client.request('foo', 'bar', 'baz')
     expect(response).toBe('the return value!')
   })
 })
 
-describe('server', () => {
+describe('Server', () => {
   it('only responds to valid requests', done => {
     expect.assertions(1)
     const parent = window
     const child = parent
     const handler = jest.fn()
-    const s = server(child, child.location.origin)(handler).start()
+    const server = new Server(child, child.location.origin, handler)
+    server.start()
     parent.addEventListener('message', function listener() {
       try {
         expect(handler.mock.calls.length).toBe(0)
-        s.stop()
         parent.removeEventListener('message', listener)
         done()
       } catch (e) {
         done.fail(e)
+      } finally {
+        server.stop()
       }
     })
     parent.postMessage('not-a-well-formed-message', parent.location.origin)
   })
 
   it('delegates to a handler to compute responses', done => {
     expect.assertions(3)
-    const testHandler = jest.fn(request =>
-      Promise.resolve({
-        id: request.id,
-        ret: 'testHandler return value'
-      })
-    )
+    const testHandler = jest.fn(async () => 'testHandler return value')
     const parent = window
     const child = parent
-    const s = server(child, child.location.origin)(testHandler).start()
+    const server = new Server(child, child.location.origin, testHandler)
+    server.start()
     child.addEventListener('message', function listener(event) {
       try {
         const request = event.data['solid-auth-client']
         const { id, ret } = request
         if (!(id && ret)) {
           return
         }
-        expect(testHandler.mock.calls[0][0]).toEqual({
-          id: '12345',
-          method: 'foo',
-          args: ['a', 'b', 'c']
-        })
+        expect(testHandler.mock.calls[0]).toEqual(['foo', 'a', 'b', 'c'])
         expect(id).toBe('12345')
         expect(ret).toBe('testHandler return value')
-        s.stop()
         child.removeEventListener('message', listener)
         done()
       } catch (e) {
         done.fail(e)
+      } finally {
+        server.stop()
       }
     })
     parent.postMessage(

src/__test__/popup.spec.js

@@ -25,44 +25,28 @@ describe('storageHandler', () => {
   it('implements getItem', async () => {
     expect.assertions(1)
     await store.setItem('foo', 'bar')
-    const resp = await handler({
-      id: '12345',
-      method: 'storage/getItem',
-      args: ['foo']
-    })
-    expect(resp).toEqual({ id: '12345', ret: 'bar' })
+    const resp = await handler('storage/getItem', 'foo')
+    expect(resp).toEqual('bar')
   })
 
   it('implements setItem', async () => {
     expect.assertions(2)
-    const resp = await handler({
-      id: '12345',
-      method: 'storage/setItem',
-      args: ['foo', 'bar']
-    })
-    expect(resp).toEqual({ id: '12345', ret: null })
+    const resp = await handler('storage/setItem', 'foo', 'bar')
+    expect(resp).toEqual(undefined)
     expect(await store.getItem('foo')).toEqual('bar')
   })
 
   it('implements removeItem', async () => {
     expect.assertions(2)
     await store.setItem('foo', 'bar')
-    const resp = await handler({
-      id: '12345',
-      method: 'storage/removeItem',
-      args: ['foo']
-    })
-    expect(resp).toEqual({ id: '12345', ret: null })
+    const resp = await handler('storage/removeItem', 'foo')
+    expect(resp).toEqual(undefined)
     expect(await store.getItem('foo')).toEqual(null)
   })
 
   it('ignores unknown methods', async () => {
     expect.assertions(1)
-    const resp = await handler({
-      id: '12345',
-      method: 'unknown_method',
-      args: ['a', 'b', 'c']
-    })
+    const resp = await handler('unknown_method', 'a', 'b', 'c')
     expect(resp).toBeNull()
   })
 })
@@ -77,17 +61,10 @@ describe('loginHandler', () => {
   it('returns the loginOptions', async () => {
     expect.assertions(1)
     const handler = loginHandler(options, () => {})
-    const _options = await handler({
-      id: '12345',
-      method: 'getLoginOptions',
-      args: []
-    })
+    const _options = await handler('getLoginOptions')
     expect(_options).toEqual({
-      id: '12345',
-      ret: {
-        popupUri: options.popupUri,
-        callbackUri: options.callbackUri
-      }
+      popupUri: options.popupUri,
+      callbackUri: options.callbackUri
     })
   })
 
@@ -99,52 +76,30 @@ describe('loginHandler', () => {
       idp: 'https://example.com',
       webId: 'https://me.example.com/profile#me'
     }
-    const _sessionResp = await handler({
-      id: '12345',
-      method: 'foundSession',
-      args: [session]
-    })
-    expect(_sessionResp).toEqual({
-      id: '12345',
-      ret: null
-    })
+    const _sessionResp = await handler('foundSession', session)
+    expect(_sessionResp).toEqual(null)
     expect(mockCallback.mock.calls.length).toBe(1)
     expect(mockCallback.mock.calls[0][0]).toEqual(session)
   })
 
   it('ignores unknown methods', async () => {
     expect.assertions(1)
     const handler = loginHandler(options, () => {})
-    const resp = await handler({
-      id: '12345',
-      method: 'unknown_method',
-      args: ['a', 'b', 'c']
-    })
+    const resp = await handler('unknown_method', 'a', 'b', 'c')
     expect(resp).toBeNull()
   })
 })
 
 describe('appOriginHandler', () => {
   it('responds with the window origin', async () => {
     expect.assertions(1)
-    const resp = await appOriginHandler({
-      id: '12345',
-      method: 'getAppOrigin',
-      args: []
-    })
-    expect(resp).toEqual({
-      id: '12345',
-      ret: 'https://app.biz'
-    })
+    const resp = await appOriginHandler('getAppOrigin')
+    expect(resp).toEqual('https://app.biz')
   })
 
   it('ignores unknown methods', async () => {
     expect.assertions(1)
-    const resp = await appOriginHandler({
-      id: '12345',
-      method: 'somethingCompletelyDifferent',
-      args: []
-    })
+    const resp = await appOriginHandler('unknown_method')
     expect(resp).toBeNull()
   })
 })