Skip to content

Commit

Permalink
fix(app): sync robot time with app time on connect (#6501)
Browse files Browse the repository at this point in the history
Closes #3872
  • Loading branch information
mcous authored Sep 10, 2020
1 parent b52ed9e commit 66dc626
Show file tree
Hide file tree
Showing 11 changed files with 247 additions and 3 deletions.
2 changes: 2 additions & 0 deletions app/src/robot-admin/__fixtures__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// mock HTTP responses for pipettes endpoints
import { mockRobot } from '../../robot-api/__fixtures__'

export * from './system-time'

export const mockRestartSuccess = {
host: mockRobot,
method: 'POST',
Expand Down
35 changes: 35 additions & 0 deletions app/src/robot-admin/__fixtures__/system-time.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// @flow
import { GET } from '../../robot-api'
import { SYSTEM_TIME_PATH } from '../constants'
import {
makeResponseFixtures,
mockFailureBody,
} from '../../robot-api/__fixtures__'

import type { SystemTimeData, SystemTimeAttributes } from '../api-types'
import type { ResponseFixtures } from '../../robot-api/__fixtures__'

export const mockSystemTimeAttributes: SystemTimeAttributes = {
systemTime: '2020-09-08T18:02:01.318292+00:00',
}

export const {
successMeta: mockFetchSystemTimeSuccessMeta,
failureMeta: mockFetchSystemTimeFailureMeta,
success: mockFetchSystemTimeSuccess,
failure: mockFetchSystemTimeFailure,
}: ResponseFixtures<
SystemTimeData,
{| message: string |}
> = makeResponseFixtures({
method: GET,
path: SYSTEM_TIME_PATH,
successStatus: 200,
successBody: {
id: 'time',
type: 'SystemTimeAttributes',
attributes: mockSystemTimeAttributes,
},
failureStatus: 500,
failureBody: mockFailureBody,
})
13 changes: 13 additions & 0 deletions app/src/robot-admin/api-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @flow

export type SystemTimeAttributes = {
systemTime: string,
...
}

export type SystemTimeData = {
id: 'time',
type: 'SystemTimeAttributes',
attributes: SystemTimeAttributes,
...
}
2 changes: 2 additions & 0 deletions app/src/robot-admin/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,5 @@ export const RESET_CONFIG_PATH: '/settings/reset' = '/settings/reset'

export const RESET_CONFIG_OPTIONS_PATH: '/settings/reset/options' =
'/settings/reset/options'

export const SYSTEM_TIME_PATH: '/system/time' = '/system/time'
127 changes: 127 additions & 0 deletions app/src/robot-admin/epic/__tests__/syncTimeOnConnectEpic.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// @flow
import { cloneDeep, set, get } from 'lodash'
import { subSeconds, differenceInSeconds, parseISO } from 'date-fns'

import { setupEpicTestMocks, runEpicTest } from '../../../robot-api/__utils__'
import { GET, PUT } from '../../../robot-api'
import { actions as RobotActions } from '../../../robot'
import {
mockFetchSystemTimeSuccess,
mockFetchSystemTimeFailure,
} from '../../__fixtures__'
import { robotAdminEpic } from '..'

const createConnectAction = robotName => (RobotActions.connect(robotName): any)

const createTimeSuccessResponse = (time: Date) => {
const response = cloneDeep(mockFetchSystemTimeSuccess)
set(response, 'body.data.attributes.systemTime', time.toISOString())
return response
}

const createEpicOutput = (mocks, createHotObservable) => {
const action$ = createHotObservable('--a', { a: mocks.action })
const state$ = createHotObservable('s-s', { s: mocks.state })
const output$ = robotAdminEpic(action$, state$)

return output$
}

describe('syncTimeOnConnectEpic', () => {
afterEach(() => {
jest.resetAllMocks()
})

it("should fetch the robot's time on connect request", () => {
const mocks = setupEpicTestMocks(
createConnectAction,
createTimeSuccessResponse(new Date())
)

runEpicTest(mocks, ({ hot, expectObservable, flush }) => {
const output$ = createEpicOutput(mocks, hot)
expectObservable(output$, '---')
flush()

expect(mocks.fetchRobotApi).toHaveBeenCalledTimes(1)
expect(mocks.fetchRobotApi).toHaveBeenCalledWith(mocks.robot, {
method: 'GET',
path: '/system/time',
})
})
})

it('should update time if off by more than a minute', () => {
const mocks = setupEpicTestMocks(createConnectAction)

runEpicTest(mocks, ({ hot, cold, expectObservable, flush }) => {
mocks.fetchRobotApi.mockImplementation((robot, request) => {
if (request.method === GET && request.path === '/system/time') {
const robotDate = subSeconds(new Date(), 61)
return cold('r', { r: createTimeSuccessResponse(robotDate) })
}

if (request.method === PUT && request.path === '/system/time') {
return cold('r', { r: createTimeSuccessResponse(new Date()) })
}

return cold('#')
})

const output$ = createEpicOutput(mocks, hot)
expectObservable(output$, '---')
flush()

expect(mocks.fetchRobotApi).toHaveBeenCalledTimes(2)
expect(mocks.fetchRobotApi).toHaveBeenNthCalledWith(2, mocks.robot, {
method: 'PUT',
path: '/system/time',
body: {
data: {
type: 'SystemTimeAttributes',
attributes: { systemTime: expect.any(String) },
},
},
})

const updatedTime = get(
mocks.fetchRobotApi.mock.calls[1][1],
'body.data.attributes.systemTime'
)

expect(
Math.abs(differenceInSeconds(new Date(), parseISO(updatedTime)))
).toBe(0)
})
})

it('should not try to update time if fetch fails', () => {
const mocks = setupEpicTestMocks(
createConnectAction,
mockFetchSystemTimeFailure
)

runEpicTest(mocks, ({ hot, expectObservable, flush }) => {
const output$ = createEpicOutput(mocks, hot)
expectObservable(output$, '---')
flush()

expect(mocks.fetchRobotApi).toHaveBeenCalledTimes(1)
})
})

it('should not try to update time if off by less than a minute', () => {
const mocks = setupEpicTestMocks(
createConnectAction,
createTimeSuccessResponse(subSeconds(new Date(), 55))
)

runEpicTest(mocks, ({ hot, cold, expectObservable, flush }) => {
const output$ = createEpicOutput(mocks, hot)
expectObservable(output$, '---')
flush()

expect(mocks.fetchRobotApi).toHaveBeenCalledTimes(1)
})
})
})
5 changes: 4 additions & 1 deletion app/src/robot-admin/epic/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import { combineEpics } from 'redux-observable'
import { restartEpic, startDiscoveryOnRestartEpic } from './restartEpic'
import { fetchResetOptionsEpic } from './fetchResetOptionsEpic'
import { resetConfigEpic, restartOnResetConfigEpic } from './resetConfigEpic'
import { syncTimeOnConnectEpic } from './syncTimeOnConnectEpic'

import type { Epic } from '../../types'

export const robotAdminEpic: Epic = combineEpics(
restartEpic,
startDiscoveryOnRestartEpic,
fetchResetOptionsEpic,
resetConfigEpic,
restartOnResetConfigEpic
restartOnResetConfigEpic,
syncTimeOnConnectEpic
)
60 changes: 60 additions & 0 deletions app/src/robot-admin/epic/syncTimeOnConnectEpic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// @flow
import { ofType } from 'redux-observable'
import { filter, map, switchMap, ignoreElements } from 'rxjs/operators'
import { parseISO, differenceInSeconds } from 'date-fns'

import { GET, PUT, fetchRobotApi } from '../../robot-api'
import { withRobotHost } from '../../robot-api/operators'
import * as Constants from '../constants'

import type { Epic } from '../../types'
import type { RobotApiRequestOptions } from '../../robot-api/types'
import type { ConnectAction } from '../../robot/actions'

const SYNC_THRESHOLD_SEC = 60

const mapActionToFetchRequest = (
action: ConnectAction
): RobotApiRequestOptions => {
return { method: GET, path: Constants.SYSTEM_TIME_PATH }
}

const createUpdateRequest = (date: Date): RobotApiRequestOptions => {
return {
method: PUT,
path: Constants.SYSTEM_TIME_PATH,
body: {
data: {
type: 'SystemTimeAttributes',
attributes: { systemTime: date.toISOString() },
},
},
}
}

export const syncTimeOnConnectEpic: Epic = (action$, state$) => {
return action$.pipe(
ofType('robot:CONNECT'),
withRobotHost(state$, action => action.payload.name),
// TODO(mc, 2020-09-08): only fetch if health.links.systemTime exists,
// see TODO in robot-server/robot_server/service/legacy/models/health.py
switchMap(([action, state, robot]) => {
const fetchSystemTimeReq = mapActionToFetchRequest(action)

return fetchRobotApi(robot, fetchSystemTimeReq).pipe(
filter(response => response.ok),
map(response => response.body.data.attributes.systemTime),
filter(systemTimeString => {
const systemTime = parseISO(systemTimeString)
const drift = differenceInSeconds(systemTime, new Date())
return Math.abs(drift) > SYNC_THRESHOLD_SEC
}),
switchMap(() => {
const updateSystemTimeReq = createUpdateRequest(new Date())
return fetchRobotApi(robot, updateSystemTimeReq)
})
)
}),
ignoreElements()
)
}
2 changes: 1 addition & 1 deletion app/src/robot-api/__utils__/epic-test-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export type EpicTestMocks<A, R> = {|
* @returns {EpicTestMocks}
*/
export const setupEpicTestMocks = <
A: { meta: { requestId: string } },
A: { meta: { requestId: string, ... }, ... },
R: RobotApiResponse
>(
makeTriggerAction: (robotName: string) => A,
Expand Down
1 change: 1 addition & 0 deletions app/src/robot-api/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

export const GET: 'GET' = 'GET'
export const POST: 'POST' = 'POST'
export const PUT: 'PUT' = 'PUT'
export const PATCH: 'PATCH' = 'PATCH'
export const DELETE: 'DELETE' = 'DELETE'

Expand Down
2 changes: 1 addition & 1 deletion app/src/robot-api/types.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @flow
import typeof { PENDING, SUCCESS, FAILURE } from './constants'

export type Method = 'GET' | 'POST' | 'PATCH' | 'DELETE'
export type Method = 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'

// api call + response types

Expand Down
1 change: 1 addition & 0 deletions robot-server/robot_server/service/legacy/models/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from pydantic import BaseModel, Field


# TODO(mc, 2020-09-08): add systemTime to health links
class Links(BaseModel):
"""A set of useful links"""
apiLog: str = \
Expand Down

0 comments on commit 66dc626

Please sign in to comment.