messages: restructure response message building

refs INTEROP-7087
flag=none

why:
* the previous commit's way of building stateless messages quickly got
unwieldy, passing around many attributes just
to use them to build a response
* because closures are Good!

test plan:
* specs still pass
* sending a postMessage to canvas still always prompts a response,
whether that is a success or an error
* and only one response, not two

Change-Id: I9c334bc90f0a4e5f32bdfc7d5f4012f4524ababb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274078
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Sean Scally <sean.scally@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Xander Moffatt <xmoffatt@instructure.com>

ui/shared/lti/jquery/__tests__/response_messages.test.js

@@ -16,28 +16,27 @@
  * with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-import {
-  sendResponse,
-  sendErrorResponse,
-  sendGenericErrorResponse,
-  sendBadRequestResponse,
-  sendWrongOriginResponse,
-  sendUnsupportedSubjectResponse,
-  sendSuccess
-} from '../response_messages'
+import buildResponseMessages from '../response_messages'
 
 describe('response_messages', () => {
   const postMessageMock = jest.fn()
   let params
+  let builder
 
   beforeEach(() => {
+    resetBuilder()
+    postMessageMock.mockRestore()
+  })
+
+  function resetBuilder(overrides = {}) {
     params = {
       targetWindow: {postMessage: postMessageMock},
       origin: 'http://tool.test',
-      subject: 'subject'
+      subject: 'subject',
+      ...overrides
     }
-    postMessageMock.mockRestore()
-  })
+    builder = buildResponseMessages(params)
+  }
 
   function expectPostMessageContents(contents, origin = expect.any(String)) {
     expect(postMessageMock).toHaveBeenCalledWith(expect.objectContaining(contents), origin)
@@ -45,7 +44,7 @@ describe('response_messages', () => {
 
   describe('sendResponse', () => {
     it('appends .response to the subject', () => {
-      sendResponse(params)
+      builder.sendResponse()
       expectPostMessageContents({subject: 'subject.response'})
     })
 
@@ -53,18 +52,18 @@ describe('response_messages', () => {
       const message_id = 'message_id'
 
       beforeEach(() => {
-        params.message_id = message_id
+        resetBuilder({message_id})
       })
 
       it('includes message_id in response', () => {
-        sendResponse(params)
+        builder.sendResponse()
         expectPostMessageContents({message_id})
       })
     })
 
     describe('when targetWindow does not exist', () => {
       beforeEach(() => {
-        delete params.targetWindow
+        resetBuilder({targetWindow: null})
         jest.spyOn(console, 'error').mockImplementation()
       })
 
@@ -74,25 +73,22 @@ describe('response_messages', () => {
       })
 
       it('logs an error to the console', () => {
-        sendResponse(params)
+        builder.sendResponse()
         // eslint-disable-next-line no-console
         expect(console.error).toHaveBeenCalled()
       })
     })
 
     it('sends response to the given origin', () => {
-      sendResponse(params)
+      builder.sendResponse()
       expectPostMessageContents({}, 'http://tool.test')
     })
 
     describe('when contents parameter is passed', () => {
       const contents = {hello: 'world'}
-      beforeEach(() => {
-        params.contents = contents
-      })
 
       it('merges contents into response', () => {
-        sendResponse(params)
+        builder.sendResponse(contents)
         expectPostMessageContents(contents)
       })
     })
@@ -100,27 +96,18 @@ describe('response_messages', () => {
 
   describe('sendSuccess', () => {
     beforeEach(() => {
-      params.message_id = 'message_id'
+      resetBuilder({message_id: 'message_id'})
     })
 
     it('only sends subject and message_id', () => {
-      sendSuccess(params)
+      builder.sendSuccess()
       expect(Object.keys(postMessageMock.mock.calls[0][0])).toEqual(['subject', 'message_id'])
     })
   })
 
   function expectCodeAndMessageInError({subject, code, message}) {
-    beforeEach(() => {
-      if (code) {
-        params.code = code
-      }
-      if (message) {
-        params.message = message
-      }
-    })
-
     it('includes code and message in error', () => {
-      subject(params)
+      subject(builder)
       const response = {}
       if (code) {
         response.code = code
@@ -132,40 +119,46 @@ describe('response_messages', () => {
     })
   }
 
-  describe('sendErrorResponse', () => {
+  describe('sendError', () => {
+    const code = 'error_code'
+    const message = 'error message'
+
     expectCodeAndMessageInError({
-      subject: sendErrorResponse,
-      code: 'error_code',
-      message: 'error message'
+      subject: builder => builder.sendError(code, message),
+      code,
+      message
     })
   })
 
-  describe('sendGenericErrorResponse', () => {
+  describe('sendGenericError', () => {
+    const message = 'generic error message'
+
     expectCodeAndMessageInError({
-      subject: sendGenericErrorResponse,
-      code: 'error',
-      message: 'generic error message'
+      subject: builder => builder.sendGenericError(message),
+      code: 'error'
     })
   })
 
-  describe('sendBadRequestResponse', () => {
+  describe('sendBadRequestError', () => {
+    const message = 'bad request error message'
+
     expectCodeAndMessageInError({
-      subject: sendBadRequestResponse,
+      subject: builder => builder.sendBadRequestError(message),
       code: 'bad_request',
-      message: 'error message'
+      message
     })
   })
 
-  describe('sendWrongOriginResponse', () => {
+  describe('sendWrongOriginError', () => {
     expectCodeAndMessageInError({
-      subject: sendWrongOriginResponse,
+      subject: builder => builder.sendWrongOriginError(),
       code: 'wrong_origin'
     })
   })
 
-  describe('sendUnsupportedSubjectResponse', () => {
+  describe('sendUnsupportedSubjectError', () => {
     expectCodeAndMessageInError({
-      subject: sendUnsupportedSubjectResponse,
+      subject: builder => builder.sendUnsupportedSubjectError(),
       code: 'unsupported_subject'
     })
   })

ui/shared/lti/jquery/messages.js

@@ -18,17 +18,12 @@
 
 /* eslint no-console: 0 */
 
-import {findDomForWindow} from './util'
 import {
   NAVIGATION_MESSAGE as MENTIONS_NAVIGATION_MESSAGE,
   INPUT_CHANGE_MESSAGE as MENTIONS_INPUT_CHANGE_MESSAGE,
   SELECTION_MESSAGE as MENTIONS_SELECTION_MESSAGE
 } from '../../rce/plugins/canvas_mentions/constants'
-import {
-  sendGenericErrorResponse,
-  sendSuccess,
-  sendUnsupportedSubjectResponse
-} from './response_messages'
+import buildResponseMessages from './response_messages'
 
 // page-global storage for data relevant to LTI postMessage events
 const ltiState = {}
@@ -71,19 +66,19 @@ async function ltiMessageHandler(e) {
 
   // look at messageType for backwards compatibility
   const subject = message.subject || message.messageType
-  const message_id = message.message_id
+  const responseMessages = buildResponseMessages({
+    targetWindow: e.source,
+    origin: e.origin,
+    subject,
+    message_id: message.message_id
+  })
 
   if (SUBJECT_IGNORE_LIST.includes(subject)) {
     // These messages are handled elsewhere
     return false
   } else if (!SUBJECT_ALLOW_LIST.includes(subject)) {
     // Enforce subject allowlist -- unknown type
-    sendUnsupportedSubjectResponse({
-      targetWindow: e.source,
-      origin: e.origin,
-      subject,
-      message_id
-    })
+    responseMessages.sendUnsupportedSubjectError()
     return false
   }
 
@@ -91,22 +86,16 @@ async function ltiMessageHandler(e) {
     const handlerModule = await import(`./subjects/${subject}.js`)
     const hasSentResponse = handlerModule.default({
       message,
-      iframe: findDomForWindow(e.source),
-      event: e
+      event: e,
+      responseMessages
     })
     if (!hasSentResponse) {
-      sendSuccess({targetWindow: e.source, origin: e.origin, subject, message_id})
+      responseMessages.sendSuccess()
     }
     return true
   } catch (error) {
     console.error(`Error loading or executing message handler for "${subject}"`, error)
-    sendGenericErrorResponse({
-      targetWindow: e.source,
-      origin: e.origin,
-      subject,
-      message_id,
-      message: error.message
-    })
+    responseMessages.sendGenericError(error.message)
     return false
   }
 }

ui/shared/lti/jquery/response_messages.js

@@ -21,66 +21,57 @@ const UNSUPPORTED_SUBJECT_ERROR_CODE = 'unsupported_subject'
 const WRONG_ORIGIN_ERROR_CODE = 'wrong_origin'
 const BAD_REQUEST_ERROR_CODE = 'bad_request'
 
-function sendResponse({targetWindow, origin, subject, message_id, contents}) {
-  const message = {subject: `${subject}.response`}
-  if (message_id) {
-    message.message_id = message_id
+const buildResponseMessages = ({targetWindow, origin, subject, message_id}) => {
+  const sendResponse = (contents = {}) => {
+    const message = {subject: `${subject}.response`}
+    if (message_id) {
+      message.message_id = message_id
+    }
+    if (targetWindow) {
+      targetWindow.postMessage({...message, ...contents}, origin)
+    } else {
+      // eslint-disable-next-line no-console
+      console.error('Error sending response postMessage: target window does not exist')
+    }
   }
-  if (targetWindow) {
-    targetWindow.postMessage({...message, ...contents}, origin)
-  } else {
-    // eslint-disable-next-line no-console
-    console.error('Error sending response postMessage: target window does not exist')
+
+  const sendSuccess = () => {
+    sendResponse({})
+  }
+
+  const sendError = (code, message) => {
+    const error = {code}
+    if (message) {
+      error.message = message
+    }
+    sendResponse({error})
+  }
+
+  const sendGenericError = message => {
+    sendError(GENERIC_ERROR_CODE, message)
+  }
+
+  const sendBadRequestError = message => {
+    sendError(BAD_REQUEST_ERROR_CODE, message)
+  }
+
+  const sendWrongOriginError = () => {
+    sendError(WRONG_ORIGIN_ERROR_CODE)
+  }
+
+  const sendUnsupportedSubjectError = () => {
+    sendError(UNSUPPORTED_SUBJECT_ERROR_CODE)
+  }
+
+  return {
+    sendResponse,
+    sendSuccess,
+    sendError,
+    sendGenericError,
+    sendBadRequestError,
+    sendWrongOriginError,
+    sendUnsupportedSubjectError
   }
 }
 
-function sendSuccess({targetWindow, origin, subject, message_id}) {
-  sendResponse({targetWindow, origin, subject, message_id, contents: {}})
-}
-
-function sendErrorResponse({targetWindow, origin, subject, message_id, code, message}) {
-  const error = {code}
-  if (message) {
-    error.message = message
-  }
-  sendResponse({targetWindow, origin, subject, message_id, contents: {error}})
-}
-
-function sendGenericErrorResponse({targetWindow, origin, subject, message_id, message}) {
-  sendErrorResponse({targetWindow, origin, subject, message_id, message, code: GENERIC_ERROR_CODE})
-}
-
-function sendBadRequestResponse({targetWindow, origin, subject, message_id, message}) {
-  sendErrorResponse({
-    targetWindow,
-    origin,
-    subject,
-    message_id,
-    message,
-    code: BAD_REQUEST_ERROR_CODE
-  })
-}
-
-function sendUnsupportedSubjectResponse({targetWindow, origin, subject, message_id}) {
-  sendErrorResponse({
-    targetWindow,
-    origin,
-    subject,
-    message_id,
-    code: UNSUPPORTED_SUBJECT_ERROR_CODE
-  })
-}
-
-function sendWrongOriginResponse({targetWindow, origin, subject, message_id}) {
-  sendErrorResponse({targetWindow, origin, subject, message_id, code: WRONG_ORIGIN_ERROR_CODE})
-}
-
-export {
-  sendResponse,
-  sendSuccess,
-  sendErrorResponse,
-  sendGenericErrorResponse,
-  sendBadRequestResponse,
-  sendUnsupportedSubjectResponse,
-  sendWrongOriginResponse
-}
+export default buildResponseMessages

ui/shared/lti/jquery/subjects/lti.enableScrollEvents.js

@@ -16,26 +16,24 @@
  * with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-export default function enableScrollEvents({iframe}) {
-  if (iframe) {
-    let timeout
-    window.addEventListener(
-      'scroll',
-      () => {
-        // requesting animation frames effectively debounces the scroll messages being sent
-        if (timeout) {
-          window.cancelAnimationFrame(timeout)
-        }
+export default function enableScrollEvents({responseMessages}) {
+  let timeout
+  window.addEventListener(
+    'scroll',
+    () => {
+      // requesting animation frames effectively debounces the scroll messages being sent
+      if (timeout) {
+        window.cancelAnimationFrame(timeout)
+      }
 
-        timeout = window.requestAnimationFrame(() => {
-          const msg = JSON.stringify({
-            subject: 'lti.scroll',
-            scrollY: window.scrollY
-          })
-          iframe.contentWindow.postMessage(msg, '*')
+      timeout = window.requestAnimationFrame(() => {
+        responseMessages.sendResponse({
+          subject: 'lti.scroll',
+          scrollY: window.scrollY
         })
-      },
-      false
-    )
-  }
+      })
+    },
+    false
+  )
+  return true
 }

ui/shared/lti/jquery/subjects/lti.fetchWindowSize.js

@@ -18,15 +18,13 @@
 
 import $ from 'jquery'
 
-export default function fetchWindowSize({message, iframe}) {
-  if (iframe) {
-    message.height = window.innerHeight
-    message.width = window.innerWidth
-    message.offset = $('.tool_content_wrapper').offset()
-    message.footer = $('#fixed_bottom').height() || 0
-    message.scrollY = window.scrollY
-    const strMessage = JSON.stringify(message)
-
-    iframe.contentWindow.postMessage(strMessage, '*')
-  }
+export default function fetchWindowSize({responseMessages}) {
+  responseMessages.sendResponse({
+    height: window.innerHeight,
+    width: window.innerWidth,
+    offset: $('.tool_content_wrapper').offset(),
+    footer: $('#fixed_bottom').height() || 0,
+    scrollY: window.scrollY
+  })
+  return true
 }

ui/shared/lti/jquery/subjects/lti.frameResize.js

@@ -17,8 +17,9 @@
  */
 
 import ToolLaunchResizer from '../tool_launch_resizer'
+import {findDomForWindow} from '../util'
 
-export default function frameResize({message, iframe, event}) {
+export default function frameResize({message, event}) {
   const toolResizer = new ToolLaunchResizer()
   let height = message.height
   if (height <= 0) height = 1
@@ -29,12 +30,15 @@ export default function frameResize({message, iframe, event}) {
   // If content.length is 0 then jquery didn't the tool wrapper.
   if (container.length > 0) {
     toolResizer.resize_tool_content_wrapper(height, container)
-  } else if (iframe) {
+  } else {
     // Attempt to find an embedded iframe that matches the event source.
-    if (typeof height === 'number') {
-      height += 'px'
+    const iframe = findDomForWindow(event.source)
+    if (iframe) {
+      if (typeof height === 'number') {
+        height += 'px'
+      }
+      iframe.height = height
+      iframe.style.height = height
     }
-    iframe.height = height
-    iframe.style.height = height
   }
 }