mirror of
https://github.com/bitwarden/browser
synced 2025-12-11 13:53:34 +00:00
[PM-20433] Add view cache options for view cache service signals to allow cached values to persist navigation events (#14348)
This commit is contained in:
@@ -27,6 +27,7 @@ import {
|
|||||||
ClEAR_VIEW_CACHE_COMMAND,
|
ClEAR_VIEW_CACHE_COMMAND,
|
||||||
POPUP_VIEW_CACHE_KEY,
|
POPUP_VIEW_CACHE_KEY,
|
||||||
SAVE_VIEW_CACHE_COMMAND,
|
SAVE_VIEW_CACHE_COMMAND,
|
||||||
|
ViewCacheState,
|
||||||
} from "../../services/popup-view-cache-background.service";
|
} from "../../services/popup-view-cache-background.service";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -42,8 +43,8 @@ export class PopupViewCacheService implements ViewCacheService {
|
|||||||
private messageSender = inject(MessageSender);
|
private messageSender = inject(MessageSender);
|
||||||
private router = inject(Router);
|
private router = inject(Router);
|
||||||
|
|
||||||
private _cache: Record<string, string>;
|
private _cache: Record<string, ViewCacheState>;
|
||||||
private get cache(): Record<string, string> {
|
private get cache(): Record<string, ViewCacheState> {
|
||||||
if (!this._cache) {
|
if (!this._cache) {
|
||||||
throw new Error("Dirty View Cache not initialized");
|
throw new Error("Dirty View Cache not initialized");
|
||||||
}
|
}
|
||||||
@@ -64,15 +65,9 @@ export class PopupViewCacheService implements ViewCacheService {
|
|||||||
filter((e) => e instanceof NavigationEnd),
|
filter((e) => e instanceof NavigationEnd),
|
||||||
/** Skip the first navigation triggered by `popupRouterCacheGuard` */
|
/** Skip the first navigation triggered by `popupRouterCacheGuard` */
|
||||||
skip(1),
|
skip(1),
|
||||||
filter((e: NavigationEnd) =>
|
|
||||||
// viewing/editing a cipher and navigating back to the vault list should not clear the cache
|
|
||||||
["/view-cipher", "/edit-cipher", "/tabs/vault"].every(
|
|
||||||
(route) => !e.urlAfterRedirects.startsWith(route),
|
|
||||||
),
|
|
||||||
),
|
|
||||||
)
|
)
|
||||||
.subscribe((e) => {
|
.subscribe(() => {
|
||||||
return this.clearState();
|
return this.clearState(true);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -85,13 +80,20 @@ export class PopupViewCacheService implements ViewCacheService {
|
|||||||
key,
|
key,
|
||||||
injector = inject(Injector),
|
injector = inject(Injector),
|
||||||
initialValue,
|
initialValue,
|
||||||
|
persistNavigation,
|
||||||
} = options;
|
} = options;
|
||||||
const cachedValue = this.cache[key] ? deserializer(JSON.parse(this.cache[key])) : initialValue;
|
const cachedValue = this.cache[key]
|
||||||
|
? deserializer(JSON.parse(this.cache[key].value))
|
||||||
|
: initialValue;
|
||||||
const _signal = signal(cachedValue);
|
const _signal = signal(cachedValue);
|
||||||
|
|
||||||
|
const viewCacheOptions = {
|
||||||
|
...(persistNavigation && { persistNavigation }),
|
||||||
|
};
|
||||||
|
|
||||||
effect(
|
effect(
|
||||||
() => {
|
() => {
|
||||||
this.updateState(key, JSON.stringify(_signal()));
|
this.updateState(key, JSON.stringify(_signal()), viewCacheOptions);
|
||||||
},
|
},
|
||||||
{ injector },
|
{ injector },
|
||||||
);
|
);
|
||||||
@@ -123,15 +125,24 @@ export class PopupViewCacheService implements ViewCacheService {
|
|||||||
return control;
|
return control;
|
||||||
}
|
}
|
||||||
|
|
||||||
private updateState(key: string, value: string) {
|
private updateState(key: string, value: string, options: ViewCacheState["options"]) {
|
||||||
this.messageSender.send(SAVE_VIEW_CACHE_COMMAND, {
|
this.messageSender.send(SAVE_VIEW_CACHE_COMMAND, {
|
||||||
key,
|
key,
|
||||||
value,
|
value,
|
||||||
|
options,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
private clearState() {
|
private clearState(routeChange: boolean = false) {
|
||||||
this._cache = {}; // clear local cache
|
if (routeChange) {
|
||||||
this.messageSender.send(ClEAR_VIEW_CACHE_COMMAND, {});
|
// Only keep entries with `persistNavigation`
|
||||||
|
this._cache = Object.fromEntries(
|
||||||
|
Object.entries(this._cache).filter(([, { options }]) => options?.persistNavigation),
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
// Clear all entries
|
||||||
|
this._cache = {};
|
||||||
|
}
|
||||||
|
this.messageSender.send(ClEAR_VIEW_CACHE_COMMAND, { routeChange: routeChange });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import {
|
|||||||
ClEAR_VIEW_CACHE_COMMAND,
|
ClEAR_VIEW_CACHE_COMMAND,
|
||||||
POPUP_VIEW_CACHE_KEY,
|
POPUP_VIEW_CACHE_KEY,
|
||||||
SAVE_VIEW_CACHE_COMMAND,
|
SAVE_VIEW_CACHE_COMMAND,
|
||||||
|
ViewCacheState,
|
||||||
} from "../../services/popup-view-cache-background.service";
|
} from "../../services/popup-view-cache-background.service";
|
||||||
|
|
||||||
import { PopupViewCacheService } from "./popup-view-cache.service";
|
import { PopupViewCacheService } from "./popup-view-cache.service";
|
||||||
@@ -35,6 +36,7 @@ export class TestComponent {
|
|||||||
signal = this.viewCacheService.signal({
|
signal = this.viewCacheService.signal({
|
||||||
key: "test-signal",
|
key: "test-signal",
|
||||||
initialValue: "initial signal",
|
initialValue: "initial signal",
|
||||||
|
persistNavigation: true,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -42,11 +44,11 @@ describe("popup view cache", () => {
|
|||||||
const configServiceMock = mock<ConfigService>();
|
const configServiceMock = mock<ConfigService>();
|
||||||
let testBed: TestBed;
|
let testBed: TestBed;
|
||||||
let service: PopupViewCacheService;
|
let service: PopupViewCacheService;
|
||||||
let fakeGlobalState: FakeGlobalState<Record<string, string>>;
|
let fakeGlobalState: FakeGlobalState<Record<string, ViewCacheState>>;
|
||||||
let messageSenderMock: MockProxy<MessageSender>;
|
let messageSenderMock: MockProxy<MessageSender>;
|
||||||
let router: Router;
|
let router: Router;
|
||||||
|
|
||||||
const initServiceWithState = async (state: Record<string, string>) => {
|
const initServiceWithState = async (state: Record<string, ViewCacheState>) => {
|
||||||
await fakeGlobalState.update(() => state);
|
await fakeGlobalState.update(() => state);
|
||||||
await service.init();
|
await service.init();
|
||||||
};
|
};
|
||||||
@@ -106,7 +108,11 @@ describe("popup view cache", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should initialize signal from state", async () => {
|
it("should initialize signal from state", async () => {
|
||||||
await initServiceWithState({ "foo-123": JSON.stringify("bar") });
|
await initServiceWithState({
|
||||||
|
"foo-123": {
|
||||||
|
value: JSON.stringify("bar"),
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
const injector = TestBed.inject(Injector);
|
const injector = TestBed.inject(Injector);
|
||||||
|
|
||||||
@@ -120,7 +126,11 @@ describe("popup view cache", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should initialize form from state", async () => {
|
it("should initialize form from state", async () => {
|
||||||
await initServiceWithState({ "test-form-cache": JSON.stringify({ name: "baz" }) });
|
await initServiceWithState({
|
||||||
|
"test-form-cache": {
|
||||||
|
value: JSON.stringify({ name: "baz" }),
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
const fixture = TestBed.createComponent(TestComponent);
|
const fixture = TestBed.createComponent(TestComponent);
|
||||||
const component = fixture.componentRef.instance;
|
const component = fixture.componentRef.instance;
|
||||||
@@ -138,7 +148,11 @@ describe("popup view cache", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should utilize deserializer", async () => {
|
it("should utilize deserializer", async () => {
|
||||||
await initServiceWithState({ "foo-123": JSON.stringify("bar") });
|
await initServiceWithState({
|
||||||
|
"foo-123": {
|
||||||
|
value: JSON.stringify("bar"),
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
const injector = TestBed.inject(Injector);
|
const injector = TestBed.inject(Injector);
|
||||||
|
|
||||||
@@ -178,6 +192,9 @@ describe("popup view cache", () => {
|
|||||||
expect(messageSenderMock.send).toHaveBeenCalledWith(SAVE_VIEW_CACHE_COMMAND, {
|
expect(messageSenderMock.send).toHaveBeenCalledWith(SAVE_VIEW_CACHE_COMMAND, {
|
||||||
key: "test-signal",
|
key: "test-signal",
|
||||||
value: JSON.stringify("Foobar"),
|
value: JSON.stringify("Foobar"),
|
||||||
|
options: {
|
||||||
|
persistNavigation: true,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -192,18 +209,63 @@ describe("popup view cache", () => {
|
|||||||
expect(messageSenderMock.send).toHaveBeenCalledWith(SAVE_VIEW_CACHE_COMMAND, {
|
expect(messageSenderMock.send).toHaveBeenCalledWith(SAVE_VIEW_CACHE_COMMAND, {
|
||||||
key: "test-form-cache",
|
key: "test-form-cache",
|
||||||
value: JSON.stringify({ name: "Foobar" }),
|
value: JSON.stringify({ name: "Foobar" }),
|
||||||
|
options: {},
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should clear on 2nd navigation", async () => {
|
it("should clear on 2nd navigation", async () => {
|
||||||
await initServiceWithState({ temp: "state" });
|
await initServiceWithState({
|
||||||
|
temp: {
|
||||||
|
value: "state",
|
||||||
|
options: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
await router.navigate(["a"]);
|
await router.navigate(["a"]);
|
||||||
expect(messageSenderMock.send).toHaveBeenCalledTimes(0);
|
expect(messageSenderMock.send).toHaveBeenCalledTimes(0);
|
||||||
expect(service["_cache"]).toEqual({ temp: "state" });
|
expect(service["_cache"]).toEqual({
|
||||||
|
temp: {
|
||||||
|
value: "state",
|
||||||
|
options: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
await router.navigate(["b"]);
|
await router.navigate(["b"]);
|
||||||
expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {});
|
expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {
|
||||||
|
routeChange: true,
|
||||||
|
});
|
||||||
expect(service["_cache"]).toEqual({});
|
expect(service["_cache"]).toEqual({});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("should respect persistNavigation setting on 2nd navigation", async () => {
|
||||||
|
await initServiceWithState({
|
||||||
|
keepState: {
|
||||||
|
value: "state",
|
||||||
|
options: {
|
||||||
|
persistNavigation: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
removeState: {
|
||||||
|
value: "state",
|
||||||
|
options: {
|
||||||
|
persistNavigation: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await router.navigate(["a"]); // first navigation covered in previous test
|
||||||
|
|
||||||
|
await router.navigate(["b"]);
|
||||||
|
expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {
|
||||||
|
routeChange: true,
|
||||||
|
});
|
||||||
|
expect(service["_cache"]).toEqual({
|
||||||
|
keepState: {
|
||||||
|
value: "state",
|
||||||
|
options: {
|
||||||
|
persistNavigation: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -16,8 +16,27 @@ import { fromChromeEvent } from "../browser/from-chrome-event";
|
|||||||
|
|
||||||
const popupClosedPortName = "new_popup";
|
const popupClosedPortName = "new_popup";
|
||||||
|
|
||||||
|
export type ViewCacheOptions = {
|
||||||
|
/**
|
||||||
|
* Optional flag to persist the cached value between navigation events.
|
||||||
|
*/
|
||||||
|
persistNavigation?: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type ViewCacheState = {
|
||||||
|
/**
|
||||||
|
* The cached value
|
||||||
|
*/
|
||||||
|
value: string; // JSON value
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Options for managing/clearing the cache
|
||||||
|
*/
|
||||||
|
options?: ViewCacheOptions;
|
||||||
|
};
|
||||||
|
|
||||||
/** We cannot use `UserKeyDefinition` because we must be able to store state when there is no active user. */
|
/** We cannot use `UserKeyDefinition` because we must be able to store state when there is no active user. */
|
||||||
export const POPUP_VIEW_CACHE_KEY = KeyDefinition.record<string>(
|
export const POPUP_VIEW_CACHE_KEY = KeyDefinition.record<ViewCacheState>(
|
||||||
POPUP_VIEW_MEMORY,
|
POPUP_VIEW_MEMORY,
|
||||||
"popup-view-cache",
|
"popup-view-cache",
|
||||||
{
|
{
|
||||||
@@ -36,9 +55,15 @@ export const POPUP_ROUTE_HISTORY_KEY = new KeyDefinition<string[]>(
|
|||||||
export const SAVE_VIEW_CACHE_COMMAND = new CommandDefinition<{
|
export const SAVE_VIEW_CACHE_COMMAND = new CommandDefinition<{
|
||||||
key: string;
|
key: string;
|
||||||
value: string;
|
value: string;
|
||||||
|
options: ViewCacheOptions;
|
||||||
}>("save-view-cache");
|
}>("save-view-cache");
|
||||||
|
|
||||||
export const ClEAR_VIEW_CACHE_COMMAND = new CommandDefinition("clear-view-cache");
|
export const ClEAR_VIEW_CACHE_COMMAND = new CommandDefinition<{
|
||||||
|
/**
|
||||||
|
* Flag to indicate the clear request was triggered by a route change in popup.
|
||||||
|
*/
|
||||||
|
routeChange: boolean;
|
||||||
|
}>("clear-view-cache");
|
||||||
|
|
||||||
export class PopupViewCacheBackgroundService {
|
export class PopupViewCacheBackgroundService {
|
||||||
private popupViewCacheState = this.globalStateProvider.get(POPUP_VIEW_CACHE_KEY);
|
private popupViewCacheState = this.globalStateProvider.get(POPUP_VIEW_CACHE_KEY);
|
||||||
@@ -61,10 +86,13 @@ export class PopupViewCacheBackgroundService {
|
|||||||
this.messageListener
|
this.messageListener
|
||||||
.messages$(SAVE_VIEW_CACHE_COMMAND)
|
.messages$(SAVE_VIEW_CACHE_COMMAND)
|
||||||
.pipe(
|
.pipe(
|
||||||
concatMap(async ({ key, value }) =>
|
concatMap(async ({ key, value, options }) =>
|
||||||
this.popupViewCacheState.update((state) => ({
|
this.popupViewCacheState.update((state) => ({
|
||||||
...state,
|
...state,
|
||||||
[key]: value,
|
[key]: {
|
||||||
|
value,
|
||||||
|
options,
|
||||||
|
},
|
||||||
})),
|
})),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
@@ -72,7 +100,19 @@ export class PopupViewCacheBackgroundService {
|
|||||||
|
|
||||||
this.messageListener
|
this.messageListener
|
||||||
.messages$(ClEAR_VIEW_CACHE_COMMAND)
|
.messages$(ClEAR_VIEW_CACHE_COMMAND)
|
||||||
.pipe(concatMap(() => this.popupViewCacheState.update(() => null)))
|
.pipe(
|
||||||
|
concatMap(({ routeChange }) =>
|
||||||
|
this.popupViewCacheState.update((state) => {
|
||||||
|
if (routeChange && state) {
|
||||||
|
// Only remove keys that are not marked with `persistNavigation`
|
||||||
|
return Object.fromEntries(
|
||||||
|
Object.entries(state).filter(([, { options }]) => options?.persistNavigation),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
)
|
||||||
.subscribe();
|
.subscribe();
|
||||||
|
|
||||||
// on popup closed, with 2 minute delay that is cancelled by re-opening the popup
|
// on popup closed, with 2 minute delay that is cancelled by re-opening the popup
|
||||||
|
|||||||
@@ -50,6 +50,7 @@ export class VaultPopupItemsService {
|
|||||||
private cachedSearchText = inject(PopupViewCacheService).signal<string>({
|
private cachedSearchText = inject(PopupViewCacheService).signal<string>({
|
||||||
key: "vault-search-text",
|
key: "vault-search-text",
|
||||||
initialValue: "",
|
initialValue: "",
|
||||||
|
persistNavigation: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
readonly searchText$ = toObservable(this.cachedSearchText);
|
readonly searchText$ = toObservable(this.cachedSearchText);
|
||||||
|
|||||||
@@ -188,6 +188,7 @@ export class VaultPopupListFiltersService {
|
|||||||
key: "vault-filters",
|
key: "vault-filters",
|
||||||
initialValue: {},
|
initialValue: {},
|
||||||
deserializer: (v) => v,
|
deserializer: (v) => v,
|
||||||
|
persistNavigation: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
this.deserializeFilters(cachedFilters());
|
this.deserializeFilters(cachedFilters());
|
||||||
|
|||||||
@@ -18,6 +18,11 @@ type BaseCacheOptions<T> = {
|
|||||||
|
|
||||||
/** An optional injector. Required if the method is called outside of an injection context. */
|
/** An optional injector. Required if the method is called outside of an injection context. */
|
||||||
injector?: Injector;
|
injector?: Injector;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Optional flag to persist the cached value between navigation events.
|
||||||
|
*/
|
||||||
|
persistNavigation?: boolean;
|
||||||
} & (T extends JsonValue ? Deserializer<T> : Required<Deserializer<T>>);
|
} & (T extends JsonValue ? Deserializer<T> : Required<Deserializer<T>>);
|
||||||
|
|
||||||
export type SignalCacheOptions<T> = BaseCacheOptions<T> & {
|
export type SignalCacheOptions<T> = BaseCacheOptions<T> & {
|
||||||
|
|||||||
Reference in New Issue
Block a user