OL2 (feat): useCache option and flag clean up (#5133)

* add addAll

* change to useCache query option which defaults to true

* add documentation

* called query params feature flags and fixed usage

* fixed debug logging

* eslint and prettier fixes

* eslint and prettier fixes

* revert

* Update packages/viewer-sandbox/src/Sandbox.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

packages/objectloader2/readme.md

@@ -77,3 +77,14 @@ When items are returned to the generator loop, `undefer` is called which caches
 A cleanup process is ran to be a singleton process. This process sorts by the total number of requests and the size. If anything falls outside the size window, then it is removed from the manager's memory cache.
 
 The aim is to speed up random access while still getting items from the cache in batches. Items that are accessed randomly tend to be references in the model.
+
+## Loader options
+
+These can be use via a query string parameter. For example: `https://app.speckle.systems/projects/57bbfabd80/models/81b8d76ef1` can have debug logging enabled with: `https://app.speckle.systems/projects/57bbfabd80/models/81b8d76ef1?debug=true`
+
+Current parameters:
+
+| Parameter  | Default | Type    |
+| ---------- | ------- | ------- |
+| `debug`    | `false` | boolean |
+| `useCache` | `true`  | boolean |

packages/objectloader2/src/core/objectLoader2.ts

@@ -34,6 +34,7 @@ export class ObjectLoader2 {
   constructor(options: ObjectLoader2Options) {
     this.#rootId = options.rootId
     this.#logger = options.logger || ((): void => {})
+    this.#logger('ObjectLoader2 initialized with rootId:', this.#rootId)
 
     const cacheOptions: CacheOptions = {
       logger: this.#logger,
@@ -56,12 +57,18 @@ export class ObjectLoader2 {
     )
     this.#deferments = new DefermentManager(this.#cache, this.#logger)
     this.#downloader = options.downloader
-    this.#cacheReader = new CacheReader(this.#database, this.#deferments, cacheOptions)
+    this.#cacheReader = new CacheReader(
+      this.#database,
+      this.#deferments,
+      this.#logger,
+      cacheOptions
+    )
     this.#cacheReader.initializeQueue(this.#gathered, this.#downloader)
     this.#cacheWriter = new CacheWriter(
       this.#database,
-      cacheOptions,
+      this.#logger,
       this.#deferments,
+      cacheOptions,
       (id: string) => {
         this.#cacheReader.requestItem(id)
       }

packages/objectloader2/src/core/objectLoader2Factory.ts

@@ -1,4 +1,4 @@
-import { CustomLogger, getQueryParameter } from '../types/functions.js'
+import { CustomLogger, getFeatureFlag, ObjectLoader2Flags } from '../types/functions.js'
 import { Base } from '../types/types.js'
 import { ObjectLoader2 } from './objectLoader2.js'
 import IndexedDatabase from './stages/indexedDatabase.js'
@@ -7,11 +7,10 @@ import { MemoryDownloader } from './stages/memory/memoryDownloader.js'
 import ServerDownloader from './stages/serverDownloader.js'
 
 export interface ObjectLoader2FactoryOptions {
-  useMemoryCache?: boolean
   // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
   keyRange?: { bound: Function; lowerBound: Function; upperBound: Function }
   indexedDB?: IDBFactory
-  logger2?: CustomLogger
+  logger?: CustomLogger
 }
 
 export class ObjectLoader2Factory {
@@ -42,46 +41,42 @@ export class ObjectLoader2Factory {
     headers?: Headers
     options?: ObjectLoader2FactoryOptions
   }): ObjectLoader2 {
-    const log = ObjectLoader2Factory.getLogger(params.options?.logger2)
-    let loader: ObjectLoader2
-    if (params.options?.useMemoryCache) {
-      loader = new ObjectLoader2({
-        rootId: params.objectId,
-        downloader: new ServerDownloader({
-          serverUrl: params.serverUrl,
-          streamId: params.streamId,
-          objectId: params.objectId,
-          token: params.token,
-          headers: params.headers
-        }),
-        database: new MemoryDatabase({
-          items: new Map<string, Base>()
-        }),
-        logger: log
+    const log = ObjectLoader2Factory.getLogger(params.options?.logger)
+    let database
+    if (getFeatureFlag(ObjectLoader2Flags.DEBUG) === 'true') {
+      this.logger('Using DEBUG mode for ObjectLoader2Factory')
+    }
+    if (getFeatureFlag(ObjectLoader2Flags.USE_CACHE) === 'true') {
+      database = new IndexedDatabase({
+        logger: log,
+        indexedDB: params.options?.indexedDB,
+        keyRange: params.options?.keyRange
       })
     } else {
-      loader = new ObjectLoader2({
-        rootId: params.objectId,
-        downloader: new ServerDownloader({
-          serverUrl: params.serverUrl,
-          streamId: params.streamId,
-          objectId: params.objectId,
-          token: params.token,
-          headers: params.headers
-        }),
-        database: new IndexedDatabase({
-          logger: log,
-          indexedDB: params.options?.indexedDB,
-          keyRange: params.options?.keyRange
-        }),
-        logger: log
+      database = new MemoryDatabase({
+        items: new Map<string, Base>()
       })
+      this.logger(
+        'Disabled persistent caching for ObjectLoader2.  Using MemoryDatabase'
+      )
     }
+    const loader = new ObjectLoader2({
+      rootId: params.objectId,
+      downloader: new ServerDownloader({
+        serverUrl: params.serverUrl,
+        streamId: params.streamId,
+        objectId: params.objectId,
+        token: params.token,
+        headers: params.headers
+      }),
+      database,
+      logger: log
+    })
     return loader
   }
 
   static getLogger(providedLogger?: CustomLogger): CustomLogger | undefined {
-    if (getQueryParameter('debug', 'false') === 'true') {
+    if (getFeatureFlag(ObjectLoader2Flags.DEBUG) === 'true') {
       return providedLogger || this.logger
     }
     return providedLogger

packages/objectloader2/src/core/stages/cacheReader.ts

@@ -18,12 +18,13 @@ export class CacheReader {
   constructor(
     database: Database,
     defermentManager: DefermentManager,
+    logger: CustomLogger,
     options: CacheOptions
   ) {
     this.#database = database
     this.#defermentManager = defermentManager
+    this.#logger = logger
     this.#options = options
-    this.#logger = options.logger || ((): void => {})
   }
 
   initializeQueue(foundQueue: Queue<Item>, notFoundQueue: Queue<string>): void {

packages/objectloader2/src/core/stages/cacheWriter.ts

@@ -17,13 +17,14 @@ export class CacheWriter implements Queue<Item> {
 
   constructor(
     database: Database,
-    options: CacheOptions,
+    logger: CustomLogger,
     defermentManager: DefermentManager,
+    options: CacheOptions,
     requestItem: (id: string) => void
   ) {
     this.#database = database
     this.#options = options
-    this.#logger = options.logger || ((): void => {})
+    this.#logger = logger
     this.#defermentManager = defermentManager
     this.#requestItem = requestItem
   }

packages/objectloader2/src/index.ts

@@ -1,3 +1,3 @@
 export { ObjectLoader2 } from './core/objectLoader2.js'
 export { ObjectLoader2Factory } from './core/objectLoader2Factory.js'
-export { getQueryParameter } from './types/functions.js'
+export { getFeatureFlag, ObjectLoader2Flags } from './types/functions.js'

packages/objectloader2/src/types/functions.spec.ts

@@ -1,5 +1,12 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
-import { isBase, isReference, isScalar, take, getQueryParameter } from './functions.js'
+import {
+  isBase,
+  isReference,
+  isScalar,
+  take,
+  getFeatureFlag,
+  ObjectLoader2Flags
+} from './functions.js'
 
 describe('isBase', () => {
   it('should return true for valid Base objects', () => {
@@ -94,11 +101,9 @@ describe('take', () => {
 })
 
 describe('getQueryParameter', () => {
-  const defaultValue = 'default'
-
   describe('in a non-browser environment', () => {
     it('should return the default value', () => {
-      expect(getQueryParameter('param', defaultValue)).toBe(defaultValue)
+      expect(getFeatureFlag(ObjectLoader2Flags.USE_CACHE)).toBe('true')
     })
   })
 
@@ -119,18 +124,18 @@ describe('getQueryParameter', () => {
     })
 
     it('should return the parameter value from the URL', () => {
-      mockWindow.location.search = '?param=value'
-      expect(getQueryParameter('param', defaultValue)).toBe('value')
+      mockWindow.location.search = '?debug=value'
+      expect(getFeatureFlag(ObjectLoader2Flags.DEBUG)).toBe('value')
     })
 
     it('should return the default value if the parameter is not in the URL', () => {
       mockWindow.location.search = '?otherparam=value'
-      expect(getQueryParameter('param', defaultValue)).toBe(defaultValue)
+      expect(getFeatureFlag(ObjectLoader2Flags.DEBUG)).toBe('false')
     })
 
     it('should return the default value if the URL has no query string', () => {
       mockWindow.location.search = ''
-      expect(getQueryParameter('param', defaultValue)).toBe(defaultValue)
+      expect(getFeatureFlag(ObjectLoader2Flags.DEBUG)).toBe('false')
     })
   })
 })

packages/objectloader2/src/types/functions.ts

@@ -50,14 +50,24 @@ export function take<T>(it: Iterator<T>, count: number): T[] {
   return result
 }
 
-export function getQueryParameter(paramName: string, defaultValue: string): string {
+export enum ObjectLoader2Flags {
+  DEBUG = 'debug',
+  USE_CACHE = 'useCache'
+}
+
+const defaultValues: Record<ObjectLoader2Flags, string> = {
+  [ObjectLoader2Flags.DEBUG]: 'false',
+  [ObjectLoader2Flags.USE_CACHE]: 'true'
+}
+
+export function getFeatureFlag(paramName: ObjectLoader2Flags): string {
   // Check if the code is running in a browser environment 🌐
   const isBrowser =
     typeof window !== 'undefined' && typeof window.document !== 'undefined'
 
   if (!isBrowser) {
     // If in Node.js or another server environment, return the default
-    return defaultValue
+    return defaultValues[paramName]
   }
 
   // In a browser, parse the query string
@@ -66,5 +76,5 @@ export function getQueryParameter(paramName: string, defaultValue: string): stri
   // .get() returns the value, or null if it's not found.
   // The nullish coalescing operator (??) provides the default value
   // if the left-hand side is null or undefined.
-  return params.get(paramName) ?? defaultValue
+  return params.get(paramName) ?? defaultValues[paramName]
 }

packages/viewer-sandbox/src/Sandbox.ts

@@ -54,7 +54,11 @@ import Bright from '../assets/hdri/Bright.png'
 import { Euler, Vector3, Box3, LinearFilter } from 'three'
 import { GeometryType } from '@speckle/viewer'
 import { MeshBatch } from '@speckle/viewer'
-import { getQueryParameter, ObjectLoader2Factory } from '@speckle/objectloader2'
+import {
+  getFeatureFlag,
+  ObjectLoader2Flags,
+  ObjectLoader2Factory
+} from '@speckle/objectloader2'
 
 export default class Sandbox {
   private viewer: Viewer
@@ -1294,6 +1298,7 @@ export default class Sandbox {
       let dataProgress = 0
       let renderedCount = 0
       let traversedCount = 0
+      const shouldLog = getFeatureFlag(ObjectLoader2Flags.DEBUG) === 'true' // means we're not already logging
       /** Too spammy */
       loader.on(LoaderEvent.LoadProgress, (arg: { progress: number; id: string }) => {
         const p = Math.floor(arg.progress * 100)
@@ -1302,12 +1307,12 @@ export default class Sandbox {
             colorImage.style.clipPath = `inset(${(1 - arg.progress) * 100}% 0 0 0)`
           dataProgress = p
 
-          if (getQueryParameter('debug', 'false') !== 'true') {
+          if (!shouldLog) {
             console.log(`Loading ${p}%`)
           }
         }
       })
-      if (getQueryParameter('debug', 'false') !== 'true') {
+      if (!shouldLog) {
         loader.on(LoaderEvent.Traversed, (arg: { count: number }) => {
           if (arg.count > traversedCount) {
             traversedCount = arg.count

packages/viewer/src/modules/loaders/Speckle/SpeckleLoader.ts

@@ -4,7 +4,8 @@ import { SpeckleGeometryConverter } from './SpeckleGeometryConverter.js'
 import { WorldTree, type SpeckleObject } from '../../../index.js'
 import Logger from '../../utils/Logger.js'
 import {
-  getQueryParameter,
+  getFeatureFlag,
+  ObjectLoader2Flags,
   ObjectLoader2,
   ObjectLoader2Factory
 } from '@speckle/objectloader2'
@@ -91,8 +92,7 @@ export class SpeckleLoader extends Loader {
       serverUrl,
       streamId,
       objectId,
-      token,
-      options: { logger2: this.log }
+      token
     })
   }
 
@@ -192,7 +192,7 @@ export class SpeckleLoader extends Loader {
   }
 
   private progressListen(): void {
-    if (getQueryParameter('debug', 'false') !== 'true') {
+    if (getFeatureFlag(ObjectLoader2Flags.DEBUG) !== 'true') {
       return
     }