feat: cache for FileProperties using Memento #3283
Conversation
packages/salesforcedx-vscode-core/src/conflict/persistentStorageService.ts
Show resolved
Hide resolved
packages/salesforcedx-vscode-core/src/conflict/persistentStorageService.ts
Show resolved
Hide resolved
| extensionContext: ExtensionContext | ||
| ) { | ||
| PersistentStorageService.initialize(extensionContext); | ||
| } |
There was a problem hiding this comment.
do we need to have a wrapper around the initialize() here? could we call initialize() directly when needed?
There was a problem hiding this comment.
Updated it to do that in the latest commit
| * Licensed under the BSD 3-Clause license. | ||
| * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
| import { FileProperties } from '@salesforce/source-deploy-retrieve/lib/src/client/types'; |
There was a problem hiding this comment.
Can we import FileProperties from the top level? ie.
import { FileProperties } from '@salesforce/source-deploy-retrieve';
There was a problem hiding this comment.
Yes, with yesterday's library release you should be able to do this.
There was a problem hiding this comment.
Would need the extensions to import SDR 2.1.5 which is currently causing me problems, perhaps related to @salesforce/core. Will update this as soon as SDR 2.1.5 is being used
|
|
||
| public static getInstance(): PersistentStorageService { | ||
| if (!PersistentStorageService.instance) { | ||
| throw new Error('Storage should have been initialized upon extension activation'); |
There was a problem hiding this comment.
Let's localize this message, we can do that by adding the message to this file
There was a problem hiding this comment.
Thanks for pointing me to that file. I made an addition, let me know if that works
|
|
||
| constructor(setGlobalState: boolean) { | ||
| this.telemetryGS = setGlobalState; | ||
| } | ||
|
|
||
| public get(key: string): any { | ||
| private getIndex(key: string): number { |
There was a problem hiding this comment.
let's just use Array.prototype.findIndex
| private static instance?: PersistentStorageService; | ||
|
|
||
| private constructor(context: ExtensionContext) { | ||
| this.storage = context.workspaceState; |
There was a problem hiding this comment.
it looks like we switched over to using the workspaceState storage but based on my local testing, I'm not seeing this storage persist data between sessions. did you find different behavior with the workspaceState storage or are there any drawbacks to using the globalState storage?
There was a problem hiding this comment.
My thought was just that using the smaller scope of workspaceState would be preferable if it did in fact work. I saw the data persist for simple cases but did not thoroughly test it, so I have switched back to global in the latest commit.
What does this PR do?
A wrapper for Memento that can serve as a cache for conflict detection.
What issues does this PR fix or reference?
@W-9258968@, @W-9275211@, @W-9275205@, @W-9275169@
Functionality Before
Mostly new things. The one place where I made edits was in the MockMemento class for unit tests. It now has functionality (implemented with arrays) rather than just method stubs.
Functionality After
PersistentStorageService class which stores LastModifiedDates using FileNames as the keys. It's methods are initialize() and getInstance() which implement a singleton pattern, a get() method, a set() method, and a setMultiple() method which proves to be useful.