unit test + some refactoring

src/components/update_notification/update_notification.js

@@ -10,7 +10,7 @@ library.add(
   faTimes
 )
 
-const CURRENT_UPDATE_COUNTER = 1
+export const CURRENT_UPDATE_COUNTER = 1
 
 const UpdateNotification = {
   data () {
@@ -40,13 +40,13 @@ const UpdateNotification = {
     },
     neverShowAgain () {
       this.toggleShow()
-      // this.$store.commit('setFlag', { flag: 'updateCounter', value: CURRENT_UPDATE_COUNTER })
+      this.$store.commit('setFlag', { flag: 'updateCounter', value: CURRENT_UPDATE_COUNTER })
-      // this.$store.commit('setFlag', { flag: 'dontShowUpdateNotifs', value: 1 })
+      this.$store.commit('setFlag', { flag: 'dontShowUpdateNotifs', value: 1 })
-      // this.$store.dispatch('pushServerSideStorage')
+      this.$store.dispatch('pushServerSideStorage')
     },
     dismiss () {
       this.$store.commit('setFlag', { flag: 'updateCounter', value: CURRENT_UPDATE_COUNTER })
-      // this.$store.dispatch('pushServerSideStorage')
+      this.$store.dispatch('pushServerSideStorage')
     }
   },
   mounted () {

src/modules/serverSideStorage.js

@@ -1,13 +1,14 @@
 import { toRaw } from 'vue'
-import { isEqual } from 'lodash'
+import { isEqual, cloneDeep } from 'lodash'
+import { CURRENT_UPDATE_COUNTER } from 'src/components/update_notification/update_notification.js'
 
 export const VERSION = 1
-export const NEW_USER_DATE = new Date('04-08-2022') // date of writing this, basically
+export const NEW_USER_DATE = new Date('2022-08-04') // date of writing this, basically
 
 export const COMMAND_TRIM_FLAGS = 1000
 export const COMMAND_TRIM_FLAGS_AND_RESET = 1001
 
-const defaultState = {
+export const defaultState = {
   // do we need to update data on server?
   dirty: false,
   // storage of flags - stuff that can only be set and incremented
@@ -27,9 +28,9 @@ const defaultState = {
   cache: null
 }
 
-const newUserFlags = {
+export const newUserFlags = {
   ...defaultState.flagStorage,
-  updateCounter: 1 // new users don't need to see update notification
+  updateCounter: CURRENT_UPDATE_COUNTER // new users don't need to see update notification
 }
 
 const _wrapData = (data) => ({
@@ -38,24 +39,23 @@ const _wrapData = (data) => ({
   _version: VERSION
 })
 
-export const _checkValidity = (data) => data._timestamp > 0 && data._version > 0
+const _checkValidity = (data) => data._timestamp > 0 && data._version > 0
 
 export const _getRecentData = (cache, live) => {
   const result = { recent: null, stale: null, needUpload: false }
   const cacheValid = _checkValidity(cache || {})
   const liveValid = _checkValidity(live || {})
-  if (!liveValid) {
+  if (!liveValid && cacheValid) {
     result.needUpload = true
     console.debug('Nothing valid stored on server, assuming cache to be source of truth')
     result.recent = cache
     result.stale = live
-  } else if (!cacheValid) {
+  } else if (!cacheValid && liveValid) {
     console.debug('Valid storage on server found, no local cache found, using live as source of truth')
     result.recent = live
     result.stale = cache
-  } else {
+  } else if (cacheValid && liveValid) {
     console.debug('Both sources have valid data, figuring things out...')
-    console.log(live._timestamp, cache._timestamp)
     if (live._timestamp === cache._timestamp && live._version === cache._version) {
       console.debug('Same version/timestamp on both source, source of truth irrelevant')
       result.recent = cache
@@ -70,14 +70,17 @@ export const _getRecentData = (cache, live) => {
         result.stale = cache
       }
     }
+  } else {
+    console.debug('Both sources are invalid, start from scratch')
+    result.needUpload = true
   }
   return result
 }
 
-export const _getAllFlags = (recent = {}, stale = {}) => {
+export const _getAllFlags = (recent, stale) => {
   return Array.from(new Set([
-    ...Object.keys(toRaw(recent.flagStorage || {})),
+    ...Object.keys(toRaw((recent || {}).flagStorage || {})),
-    ...Object.keys(toRaw(stale.flagStorage || {}))
+    ...Object.keys(toRaw((stale || {}).flagStorage || {}))
   ]))
 }
 
@@ -86,37 +89,43 @@ export const _mergeFlags = (recent, stale, allFlagKeys) => {
     const recentFlag = recent.flagStorage[flag]
     const staleFlag = stale.flagStorage[flag]
     // use flag that is of higher value
-    return [flag, recentFlag > staleFlag ? recentFlag : staleFlag]
+    return [flag, Number((recentFlag > staleFlag ? recentFlag : staleFlag) || 0)]
   }))
 }
 
-export const _resetFlags = (totalFlags, allFlagKeys) => {
+export const _resetFlags = (totalFlags, knownKeys = defaultState.flagStorage) => {
+  let result = { ...totalFlags }
+  const allFlagKeys = Object.keys(totalFlags)
   // flag reset functionality
   if (totalFlags.reset >= COMMAND_TRIM_FLAGS && totalFlags.reset <= COMMAND_TRIM_FLAGS_AND_RESET) {
     console.debug('Received command to trim the flags')
-    const knownKeys = new Set(Object.keys(defaultState.flagStorage))
+    const knownKeysSet = new Set(Object.keys(knownKeys))
+
+    // Trim
+    result = {}
     allFlagKeys.forEach(flag => {
-      if (!knownKeys.has(flag)) {
+      if (knownKeysSet.has(flag)) {
-        delete totalFlags[flag]
+        result[flag] = totalFlags[flag]
       }
     })
+
+    // Reset
     if (totalFlags.reset === COMMAND_TRIM_FLAGS_AND_RESET) {
       // 1001 - and reset everything to 0
       console.debug('Received command to reset the flags')
-      allFlagKeys.forEach(flag => { totalFlags[flag] = 0 })
+      Object.keys(knownKeys).forEach(flag => { result[flag] = 0 })
-    } else {
-      // reset the reset 0
-      totalFlags.reset = 0
     }
   } else if (totalFlags.reset > 0 && totalFlags.reset < 9000) {
     console.debug('Received command to reset the flags')
-    allFlagKeys.forEach(flag => { totalFlags[flag] = 0 })
+    allFlagKeys.forEach(flag => { result[flag] = 0 })
-    // for good luck
-    totalFlags.reset = 0
   }
+  result.reset = 0
+  return result
 }
 
 export const _doMigrations = (cache) => {
+  if (!cache) return cache
+
   if (cache._version < VERSION) {
     console.debug('Local cached data has older version, seeing if there any migrations that can be applied')
 
@@ -139,65 +148,69 @@ export const _doMigrations = (cache) => {
   return cache
 }
 
+export const mutations = {
+  setServerSideStorage (state, userData) {
+    const live = userData.storage
+    state.raw = live
+    let cache = state.cache
+
+    cache = _doMigrations(cache)
+
+    let { recent, stale, needsUpload } = _getRecentData(cache, live)
+
+    const userNew = userData.created_at > NEW_USER_DATE
+    const flagsTemplate = userNew ? newUserFlags : defaultState.flagStorage
+    let dirty = false
+
+    if (recent === null) {
+      console.debug(`Data is empty, initializing for ${userNew ? 'new' : 'existing'} user`)
+      recent = _wrapData({
+        flagStorage: { ...flagsTemplate }
+      })
+    }
+
+    if (!needsUpload && recent && stale) {
+      console.debug('Checking if data needs merging...')
+      // discarding timestamps and versions
+      const { _timestamp: _0, _version: _1, ...recentData } = recent
+      const { _timestamp: _2, _version: _3, ...staleData } = stale
+      dirty = !isEqual(recentData, staleData)
+      console.debug(`Data ${dirty ? 'needs' : 'doesn\'t need'} merging`)
+    }
+
+    const allFlagKeys = _getAllFlags(recent, stale)
+    let totalFlags
+    if (dirty) {
+      // Merge the flags
+      console.debug('Merging the flags...')
+      totalFlags = _mergeFlags(recent, stale, allFlagKeys)
+    } else {
+      totalFlags = recent.flagStorage
+    }
+
+    totalFlags = _resetFlags(totalFlags)
+
+    recent.flagStorage = totalFlags
+
+    state.dirty = dirty || needsUpload
+    state.cache = recent
+    // set local timestamp to smaller one if we don't have any changes
+    if (stale && recent && !state.dirty) {
+      state.cache._timestamp = Math.min(stale._timestamp, recent._timestamp)
+    }
+    state.flagStorage = state.cache.flagStorage
+  },
+  setFlag (state, { flag, value }) {
+    state.flagStorage[flag] = value
+    state.dirty = true
+  }
+}
+
 const serverSideStorage = {
   state: {
-    ...defaultState
+    ...cloneDeep(defaultState)
-  },
-  mutations: {
-    setServerSideStorage (state, userData) {
-      const live = userData.storage
-      state.raw = live
-      let cache = state.cache
-
-      cache = _doMigrations(cache)
-
-      let { recent, stale, needsUpload } = _getRecentData(cache, live)
-
-      const userNew = userData.created_at > NEW_USER_DATE
-      const flagsTemplate = userNew ? newUserFlags : defaultState.defaultState
-      let dirty = false
-
-      if (recent === null) {
-        console.debug(`Data is empty, initializing for ${userNew ? 'new' : 'existing'} user`)
-        recent = _wrapData({
-          flagStorage: { ...flagsTemplate }
-        })
-      }
-
-      if (!needsUpload && recent && stale) {
-        console.debug('Checking if data needs merging...')
-        // discarding timestamps and versions
-        const { _timestamp: _0, _version: _1, ...recentData } = recent
-        const { _timestamp: _2, _version: _3, ...staleData } = stale
-        dirty = !isEqual(recentData, staleData)
-        console.debug(`Data ${dirty ? 'needs' : 'doesn\'t need'} merging`)
-      }
-
-      const allFlagKeys = _getAllFlags(recent, stale)
-      let totalFlags
-      if (dirty) {
-        // Merge the flags
-        console.debug('Merging the flags...')
-        totalFlags = _mergeFlags(recent, stale, allFlagKeys)
-      } else {
-        totalFlags = recent.flagStorage
-      }
-
-      // This does side effects on totalFlags !!!
-      // only resets if needed (checks are inside)
-      _resetFlags(totalFlags, allFlagKeys)
-
-      recent.flagStorage = totalFlags
-
-      state.dirty = dirty || needsUpload
-      state.cache = recent
-      state.flagStorage = state.cache.flagStorage
-    },
-    setFlag (state, { flag, value }) {
-      state.flagStorage[flag] = value
-      state.dirty = true
-    }
   },
+  mutations,
   actions: {
     pushServerSideStorage ({ state, rootState, commit }, { force = false } = {}) {
       const needPush = state.dirty || force

test/unit/specs/modules/serverSideStorage.spec.js

@@ -0,0 +1,178 @@
+import { cloneDeep } from 'lodash'
+
+import {
+  VERSION,
+  COMMAND_TRIM_FLAGS,
+  COMMAND_TRIM_FLAGS_AND_RESET,
+  _getRecentData,
+  _getAllFlags,
+  _mergeFlags,
+  _resetFlags,
+  mutations,
+  defaultState,
+  newUserFlags
+} from 'src/modules/serverSideStorage.js'
+
+describe('The serverSideStorage module', () => {
+  describe('mutations', () => {
+    describe('setServerSideStorage', () => {
+      const { setServerSideStorage } = mutations
+      const user = {
+        created_at: new Date('1999-02-09'),
+        storage: {}
+      }
+
+      it('should initialize storage if none present', () => {
+        const state = cloneDeep(defaultState)
+        setServerSideStorage(state, user)
+        expect(state.cache._version).to.eql(VERSION)
+        expect(state.cache._timestamp).to.be.a('number')
+        expect(state.cache.flagStorage).to.eql(defaultState.flagStorage)
+      })
+
+      it('should initialize storage with proper flags for new users if none present', () => {
+        const state = cloneDeep(defaultState)
+        setServerSideStorage(state, { ...user, created_at: new Date() })
+        expect(state.cache._version).to.eql(VERSION)
+        expect(state.cache._timestamp).to.be.a('number')
+        expect(state.cache.flagStorage).to.eql(newUserFlags)
+      })
+
+      it('should merge flags even if remote timestamp is older', () => {
+        const state = {
+          ...cloneDeep(defaultState),
+          cache: {
+            _timestamp: Date.now(),
+            _version: VERSION,
+            ...cloneDeep(defaultState)
+          }
+        }
+        setServerSideStorage(
+          state,
+          {
+            ...user,
+            storage: {
+              _timestamp: 123,
+              _version: VERSION,
+              flagStorage: {
+                ...defaultState.flagStorage,
+                updateCounter: 1
+              }
+            }
+          }
+        )
+        expect(state.cache.flagStorage).to.eql({
+          ...defaultState.flagStorage,
+          updateCounter: 1
+        })
+      })
+
+      it('should reset local timestamp to remote if contents are the same', () => {
+        const state = {
+          ...cloneDeep(defaultState),
+          cache: null
+        }
+        setServerSideStorage(
+          state,
+          {
+            ...user,
+            storage: {
+              _timestamp: 123,
+              _version: VERSION,
+              flagStorage: {
+                ...defaultState.flagStorage,
+                updateCounter: 999
+              }
+            }
+          }
+        )
+        expect(state.cache._timestamp).to.eql(123)
+        expect(state.flagStorage.updateCounter).to.eql(999)
+        expect(state.cache.flagStorage.updateCounter).to.eql(999)
+      })
+
+      it('should remote version if local missing', () => {
+        const state = cloneDeep(defaultState)
+        setServerSideStorage(state, user)
+        expect(state.cache._version).to.eql(VERSION)
+        expect(state.cache._timestamp).to.be.a('number')
+        expect(state.cache.flagStorage).to.eql(defaultState.flagStorage)
+      })
+    })
+  })
+
+  describe('helper functions', () => {
+    describe('_getRecentData', () => {
+      it('should handle nulls correctly', () => {
+        expect(_getRecentData(null, null)).to.eql({ recent: null, stale: null, needUpload: true })
+      })
+
+      it('doesn\'t choke on invalid data', () => {
+        expect(_getRecentData({ a: 1 }, { b: 2 })).to.eql({ recent: null, stale: null, needUpload: true })
+      })
+
+      it('should prefer the valid non-null correctly, needUpload works properly', () => {
+        const nonNull = { _version: VERSION, _timestamp: 1 }
+        expect(_getRecentData(nonNull, null)).to.eql({ recent: nonNull, stale: null, needUpload: true })
+        expect(_getRecentData(null, nonNull)).to.eql({ recent: nonNull, stale: null, needUpload: false })
+      })
+
+      it('should prefer the one with higher timestamp', () => {
+        const a = { _version: VERSION, _timestamp: 1 }
+        const b = { _version: VERSION, _timestamp: 2 }
+
+        expect(_getRecentData(a, b)).to.eql({ recent: b, stale: a, needUpload: false })
+        expect(_getRecentData(b, a)).to.eql({ recent: b, stale: a, needUpload: false })
+      })
+
+      it('case where both are same', () => {
+        const a = { _version: VERSION, _timestamp: 3 }
+        const b = { _version: VERSION, _timestamp: 3 }
+
+        expect(_getRecentData(a, b)).to.eql({ recent: b, stale: a, needUpload: false })
+        expect(_getRecentData(b, a)).to.eql({ recent: b, stale: a, needUpload: false })
+      })
+    })
+
+    describe('_getAllFlags', () => {
+      it('should handle nulls properly', () => {
+        expect(_getAllFlags(null, null)).to.eql([])
+      })
+      it('should output list of keys if passed single object', () => {
+        expect(_getAllFlags({ flagStorage: { a: 1, b: 1, c: 1 } }, null)).to.eql(['a', 'b', 'c'])
+      })
+      it('should union keys of both objects', () => {
+        expect(_getAllFlags({ flagStorage: { a: 1, b: 1, c: 1 } }, { flagStorage: { c: 1, d: 1 } })).to.eql(['a', 'b', 'c', 'd'])
+      })
+    })
+
+    describe('_mergeFlags', () => {
+      it('should handle merge two flag sets correctly picking higher numbers', () => {
+        expect(
+          _mergeFlags(
+            { flagStorage: { a: 0, b: 3 } },
+            { flagStorage: { b: 1, c: 4, d: 9 } },
+            ['a', 'b', 'c', 'd'])
+        ).to.eql({ a: 0, b: 3, c: 4, d: 9 })
+      })
+    })
+
+    describe('_resetFlags', () => {
+      it('should reset all known flags to 0 when reset flag is set to > 0 and < 9000', () => {
+        const totalFlags = { a: 0, b: 3, reset: 1 }
+
+        expect(_resetFlags(totalFlags)).to.eql({ a: 0, b: 0, reset: 0 })
+      })
+      it('should trim all flags to known when reset is set to 1000', () => {
+        const totalFlags = { a: 0, b: 3, c: 33, reset: COMMAND_TRIM_FLAGS }
+
+        expect(_resetFlags(totalFlags, { a: 0, b: 0, reset: 0 })).to.eql({ a: 0, b: 3, reset: 0 })
+      })
+      it('should trim all flags to known and reset when reset is set to 1001', () => {
+        const totalFlags = { a: 0, b: 3, c: 33, reset: COMMAND_TRIM_FLAGS_AND_RESET }
+
+        expect(_resetFlags(totalFlags, { a: 0, b: 0, reset: 0 })).to.eql({ a: 0, b: 0, reset: 0 })
+      })
+    })
+  })
+})