feat(app, app-shell): add ability to disable discovery cache (#5759)

app-shell/src/__tests__/discovery.test.js

@@ -4,7 +4,7 @@ import { app } from 'electron'
 import Store from 'electron-store'
 import { createDiscoveryClient } from '@opentrons/discovery-client'
 import { registerDiscovery } from '../discovery'
-import { getConfig, getOverrides } from '../config'
+import { getConfig, getOverrides, handleConfigChange } from '../config'
 
 jest.mock('electron')
 jest.mock('electron-store')
@@ -259,4 +259,55 @@ describe('app-shell/discovery', () => {
       payload: { robots: [] },
     })
   })
+
+  it('does not update services from store when caching disabled', () => {
+    // cache has been disabled
+    getConfig.mockReturnValue({
+      candidates: [],
+      disableCache: true,
+    })
+    // discovery.json contains 1 entry
+    Store.__store.get.mockImplementation(key => {
+      if (key === 'services') return [{ name: 'foo' }]
+      return null
+    })
+
+    registerDiscovery(dispatch)
+
+    // should not contain above entry
+    expect(createDiscoveryClient).toHaveBeenCalledWith(
+      expect.objectContaining({
+        services: [],
+      })
+    )
+  })
+
+  it('clears cache & suspends caching when caching changes to disabled', () => {
+    // Cache enabled initially
+    getConfig.mockReturnValue({
+      candidates: [],
+      disableCache: false,
+    })
+    // discovery.json contains 1 entry
+    Store.__store.get.mockImplementation(key => {
+      if (key === 'services') return [{ name: 'foo' }]
+      return null
+    })
+
+    registerDiscovery(dispatch)
+
+    // the 'discovery.disableCache' change handler
+    const changeHandler = handleConfigChange.mock.calls[1][1]
+    const disableCache = true
+    changeHandler(disableCache)
+
+    expect(Store.__store.set).toHaveBeenCalledWith('services', [])
+
+    // new services discovered
+    mockClient.services = [{ name: 'foo' }, { name: 'bar' }]
+    mockClient.emit('service')
+
+    // but discovery.json should not update
+    expect(Store.__store.set).toHaveBeenLastCalledWith('services', [])
+  })
 })

app-shell/src/discovery.js

@@ -33,12 +33,13 @@ export function registerDiscovery(dispatch: Dispatch): Action => mixed {
 
   config = getConfig('discovery')
   store = new Store({ name: 'discovery', defaults: { services: [] } })
+  let disableCache = config.disableCache
 
   client = createDiscoveryClient({
     pollInterval: SLOW_POLL_INTERVAL_MS,
     logger: log,
     candidates: ['[fd00:0:cafe:fefe::1]'].concat(config.candidates),
-    services: store.get('services'),
+    services: disableCache ? [] : store.get('services'),
   })
 
   client
@@ -51,6 +52,13 @@ export function registerDiscovery(dispatch: Dispatch): Action => mixed {
     client.setCandidates(['[fd00:0:cafe:fefe::1]'].concat(value))
   )
 
+  handleConfigChange('discovery.disableCache', value => {
+    if (value === true) {
+      clearCache()
+    }
+    disableCache = value
+  })
+
   app.once('will-quit', () => client.stop())
 
   return function handleIncomingAction(action: Action) {
@@ -73,7 +81,9 @@ export function registerDiscovery(dispatch: Dispatch): Action => mixed {
   }
 
   function handleServices() {
-    store.set('services', filterServicesToPersist(client.services))
+    if (!disableCache) {
+      store.set('services', filterServicesToPersist(client.services))
+    }
     dispatch({
       type: 'discovery:UPDATE_LIST',
       payload: { robots: client.services },

app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js

@@ -0,0 +1,31 @@
+// @flow
+import * as React from 'react'
+import { useSelector, useDispatch } from 'react-redux'
+import { LabeledToggle } from '@opentrons/components'
+import type { State, Dispatch } from '../../types'
+import { getConfig, updateConfig } from '../../config'
+
+export const DisableDiscoveryCache = () => {
+  const cacheDisabled = useSelector((state: State) => {
+    const config = getConfig(state)
+    return config.discovery.disableCache
+  })
+  const dispatch = useDispatch<Dispatch>()
+  return (
+    <LabeledToggle
+      label="Disable robot caching"
+      toggledOn={cacheDisabled}
+      onClick={() => {
+        dispatch(updateConfig('discovery.disableCache', !cacheDisabled))
+      }}
+    >
+      <p>NOTE: This will clear cached robots when switched ON.</p>
+      <p>
+        Disable caching of previously seen robots. Enabling this setting may
+        improve overall networking performance in environments with many OT-2s,
+        but may cause initial OT-2 discovery on app launch to be slower and more
+        susceptible to failures.
+      </p>
+    </LabeledToggle>
+  )
+}

app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js

@@ -0,0 +1,91 @@
+// @flow
+import * as React from 'react'
+import { Provider } from 'react-redux'
+import { mount } from 'enzyme'
+import noop from 'lodash/noop'
+
+import * as AllConfig from '../../../config'
+import { LabeledToggle } from '@opentrons/components'
+import { DisableDiscoveryCache } from '../DisableDiscoveryCache'
+
+import type { State } from '../../../types'
+import type { Config } from '../../../config/types'
+
+jest.mock('../../../config/selectors')
+
+const MOCK_STATE: State = ({ mockState: true }: any)
+
+const getConfig: JestMockFn<[State], $Shape<Config>> = AllConfig.getConfig
+
+function stubSelector<R>(mock: JestMockFn<[State], R>, rVal: R) {
+  mock.mockImplementation(state => {
+    expect(state).toBe(MOCK_STATE)
+    return rVal
+  })
+}
+
+describe('DisableDiscoveryCache', () => {
+  const dispatch = jest.fn()
+  const MOCK_STORE = {
+    dispatch: dispatch,
+    subscribe: noop,
+    getState: () => MOCK_STATE,
+  }
+
+  const render = () => {
+    return mount(<DisableDiscoveryCache />, {
+      wrappingComponent: Provider,
+      wrappingComponentProps: { store: MOCK_STORE },
+    })
+  }
+
+  beforeEach(() => {
+    stubSelector(getConfig, {
+      discovery: { candidates: [], disableCache: false },
+    })
+  })
+
+  afterEach(() => {
+    jest.resetAllMocks()
+  })
+
+  it('renders a labelled toggle component', () => {
+    const wrapper = render()
+    const theToggle = wrapper.find(LabeledToggle)
+    expect(theToggle.prop('label')).toBe('Disable robot caching')
+    expect(theToggle.prop('toggledOn')).toBe(false)
+  })
+
+  it('renders description of the toggle component', () => {
+    const wrapper = render()
+    expect(wrapper.children().html()).toMatch(
+      /Disable caching of previously seen robots./
+    )
+  })
+
+  it('updates the toggle status according to disableCache config', () => {
+    stubSelector(getConfig, {
+      discovery: { candidates: [], disableCache: true },
+    })
+    const wrapper = render()
+    expect(wrapper.find(LabeledToggle).prop('toggledOn')).toBe(true)
+
+    // toggle switches value
+    stubSelector(getConfig, {
+      discovery: { candidates: [], disableCache: false },
+    })
+
+    // trigger a re-render
+    wrapper.setProps({})
+    expect(wrapper.find(LabeledToggle).prop('toggledOn')).toBe(false)
+  })
+
+  it('dispatches config update on toggle', () => {
+    const wrapper = render()
+    const theToggle = wrapper.find(LabeledToggle)
+    theToggle.prop('onClick')()
+    expect(dispatch).toHaveBeenCalledWith(
+      AllConfig.updateConfig('discovery.disableCache', true)
+    )
+  })
+})

app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js

@@ -6,6 +6,7 @@ import { Card } from '@opentrons/components'
 import { AddManualIp } from '../AddManualIp'
 import { NetworkSettingsCard } from '..'
 import { ClearDiscoveryCache } from '../ClearDiscoveryCache'
+import { DisableDiscoveryCache } from '../DisableDiscoveryCache'
 
 describe('NetworkSettingsCard', () => {
   it('should render a card with the proper title', () => {
@@ -22,4 +23,9 @@ describe('NetworkSettingsCard', () => {
     const wrapper = shallow(<NetworkSettingsCard />)
     expect(wrapper.exists(ClearDiscoveryCache)).toBe(true)
   })
+
+  it('should render a DisableDiscoveryCache component', () => {
+    const wrapper = shallow(<NetworkSettingsCard />)
+    expect(wrapper.exists(DisableDiscoveryCache)).toBe(true)
+  })
 })

app/src/components/NetworkSettingsCard/index.js

@@ -4,13 +4,15 @@ import * as React from 'react'
 import { Card } from '@opentrons/components'
 import { AddManualIp } from './AddManualIp'
 import { ClearDiscoveryCache } from './ClearDiscoveryCache'
+import { DisableDiscoveryCache } from './DisableDiscoveryCache'
 
 // TODO(mc, 2020-04-27): i18n
 const NETWORK_SETTINGS = 'Network Settings'
 
 export const NetworkSettingsCard = (): React.Node => (
   <Card title={NETWORK_SETTINGS}>
     <AddManualIp />
+    <DisableDiscoveryCache />
     <ClearDiscoveryCache />
   </Card>
 )