From e82fc2ae38b2484a7ac59c83b5fa9540e8ed4aa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sch=C3=B6dler?= Date: Sat, 20 May 2023 21:43:08 +0200 Subject: [PATCH] Refactors cache to allow identical cachekeys, adds indexingService tests --- src/services/indexingService.test.ts | 105 ++++++++++++++------------- src/services/indexingService.ts | 22 ++---- src/services/suggestionsService.ts | 91 ++++++++++++----------- src/view/treeItem.ts | 8 ++ src/view/viewBuilder.ts | 1 + styles.css | 8 +- 6 files changed, 125 insertions(+), 110 deletions(-) diff --git a/src/services/indexingService.test.ts b/src/services/indexingService.test.ts index 28c433c..959ebc8 100644 --- a/src/services/indexingService.test.ts +++ b/src/services/indexingService.test.ts @@ -11,7 +11,7 @@ // GNU General Public License for more details. import { CachedMetadata, FileStats, HeadingCache, TFile, TFolder, TagCache, Vault } from 'obsidian'; -import { CrossbowIndexingService } from './indexingService'; +import { CrossbowIndexingService, SourceCacheEntryLookupMap } from './indexingService'; import { CrossbowLoggingService } from './loggingService'; import { CrossbowSettingsService, DEFAULT_SETTINGS } from './settingsService'; @@ -26,19 +26,17 @@ describe('indexingService', () => { const service = createServiceMock(); service.indexFile(file, metadata); - const cache = service.getCache(); + const fileCache = service.getCache()[file.path]; // Key count - expect(Object.keys(cache)).toHaveLength(5); - expect(Object.values(cache).filter((value) => value.type === 'File')).toHaveLength(1); - expect(Object.values(cache).filter((value) => value.type === 'Heading')).toHaveLength(3); - expect(Object.values(cache).filter((value) => value.type === 'Tag')).toHaveLength(1); - - expect(Object.keys(getSourceLookup(service))).toHaveLength(1); + expect(Object.keys(fileCache)).toHaveLength(5); + expect(Object.values(fileCache).filter((value) => value.type === 'File')).toHaveLength(1); + expect(Object.values(fileCache).filter((value) => value.type === 'Heading')).toHaveLength(3); + expect(Object.values(fileCache).filter((value) => value.type === 'Tag')).toHaveLength(1); // Should contain file cache entry - expect(cache[fileName]).toBeDefined(); - expect(cache[fileName].type).toBe('File'); + expect(fileCache[fileName]).toBeDefined(); + expect(fileCache[fileName].type).toBe('File'); if (metadata.headings === undefined) { throw new Error('Metadata headings are undefined'); @@ -46,10 +44,10 @@ describe('indexingService', () => { // Should contain heading cache entries metadata.headings.forEach((headingCache) => { - expect(cache[headingCache.heading]).toBeDefined(); - expect(cache[headingCache.heading].type).toBe('Heading'); - expect(cache[headingCache.heading].file).toBe(file); - expect(cache[headingCache.heading].text).toBe(headingCache.heading); + expect(fileCache[headingCache.heading]).toBeDefined(); + expect(fileCache[headingCache.heading].type).toBe('Heading'); + expect(fileCache[headingCache.heading].file).toBe(file); + expect(fileCache[headingCache.heading].text).toBe(headingCache.heading); }); if (metadata.tags === undefined) { @@ -58,10 +56,10 @@ describe('indexingService', () => { // Should contain tag cache entries metadata.tags.forEach((tagCache) => { - expect(cache[tagCache.tag]).toBeDefined(); - expect(cache[tagCache.tag].type).toBe('Tag'); - expect(cache[tagCache.tag].file).toBe(file); - expect(cache[tagCache.tag].text).toBe(tagCache.tag); + expect(fileCache[tagCache.tag]).toBeDefined(); + expect(fileCache[tagCache.tag].type).toBe('Tag'); + expect(fileCache[tagCache.tag].file).toBe(file); + expect(fileCache[tagCache.tag].text).toBe(tagCache.tag); }); }); @@ -71,10 +69,10 @@ describe('indexingService', () => { const service = createServiceMock(); service.indexFile(file, metadata); - expect(Object.keys(service.getCache())).toHaveLength(5); + expect(Object.keys(service.getCache()[file.path])).toHaveLength(5); service.indexFile(file, metadata); - expect(Object.keys(service.getCache())).toHaveLength(5); + expect(Object.keys(service.getCache()[file.path])).toHaveLength(5); }); }); @@ -85,31 +83,15 @@ describe('indexingService', () => { const service = createServiceMock(); service.indexFile(file, metadata); - expect(Object.keys(service.getCache())).toHaveLength(5); - expect(Object.keys(getSourceLookup(service))).toHaveLength(1); + expect(Object.keys(service.getCache()[file.path])).toHaveLength(5); service.clearCacheFromFile(file); - expect(Object.keys(service.getCache())).toHaveLength(0); - expect(Object.keys(getSourceLookup(service))).toHaveLength(0); + + expect(service.getCache()[file.path]).toBeUndefined(); + expect(Object.keys(Object.keys(service.getCache()).filter((filePath) => filePath === file.path))).toHaveLength(0); }); it("shouldn't clear cache entries (headings, tags and the file) of other files", () => { - const file = createFileMock('testFile'); - const metadata = createMetadataCacheMock(); - const service = createServiceMock(); - - service.indexFile(file, metadata); - expect(Object.keys(service.getCache())).toHaveLength(5); - expect(Object.keys(getSourceLookup(service))).toHaveLength(1); - - service.clearCacheFromFile(file); - expect(Object.keys(service.getCache())).toHaveLength(0); - expect(Object.keys(getSourceLookup(service))).toHaveLength(0); - }); - }); - - describe(`${proto.clearCache.name}()`, () => { - it('should clear cache (and sourceFileLookup)', () => { const fileName1 = 'testFile'; const file1 = createFileMock(fileName1); const metadata1 = createMetadataCacheMock(); @@ -123,12 +105,38 @@ describe('indexingService', () => { service.indexFile(file1, metadata1); service.indexFile(file2, metadata2); - expect(Object.keys(service.getCache())).toHaveLength(10); - expect(Object.keys(getSourceLookup(service))).toHaveLength(2); + expect(countCacheEntries(service.getCache())).toEqual(10); + expect(getCacheSources(service.getCache())).toEqual([file1.path, file2.path]); + + service.clearCacheFromFile(file1); + + expect(countCacheEntries(service.getCache())).toEqual(5); + expect(getCacheSources(service.getCache())).toEqual([file2.path]); + }); + }); + + describe(`${proto.clearCache.name}()`, () => { + it('should clear cache', () => { + const fileName1 = 'testFile'; + const file1 = createFileMock(fileName1); + const metadata1 = createMetadataCacheMock(); + + const fileName2 = 'testFile2'; + const file2 = createFileMock(fileName2); + const metadata2 = createMetadataCacheMock(); + + const service = createServiceMock(); + + service.indexFile(file1, metadata1); + service.indexFile(file2, metadata2); + + expect(countCacheEntries(service.getCache())).toEqual(10); + expect(getCacheSources(service.getCache())).toEqual([file1.path, file2.path]); service.clearCache(); - expect(Object.keys(service.getCache())).toHaveLength(5); - expect(Object.keys(getSourceLookup(service))).toHaveLength(1); + + expect(countCacheEntries(service.getCache())).toEqual(0); + expect(getCacheSources(service.getCache())).toEqual([]); }); }); }); @@ -141,11 +149,6 @@ settingsServiceMock.saveSettings(DEFAULT_SETTINGS); const createServiceMock = (): CrossbowIndexingService => new CrossbowIndexingService(settingsServiceMock, loggingService); -const getSourceLookup = (service: CrossbowIndexingService): object => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (service as any).sourceFileLookup as object; -}; - const createFileMock = (fileName: string): TFile => ({ basename: fileName, name: `${fileName}Name`, @@ -182,3 +185,7 @@ const createMetadataCacheMock = (headingsCount = 3, tagsCount = 1): CachedMetada return { headings, tags }; }; + +const getCacheSources = (cache: SourceCacheEntryLookupMap): string[] => Object.keys(cache); +const countCacheEntries = (cache: SourceCacheEntryLookupMap): number => + Object.values(cache).reduce((a, b) => a + Object.values(b).length, 0); diff --git a/src/services/indexingService.ts b/src/services/indexingService.ts index 88e78fe..6aecbc3 100644 --- a/src/services/indexingService.ts +++ b/src/services/indexingService.ts @@ -15,7 +15,7 @@ import { CrossbowLoggingService } from './loggingService'; import { CrossbowSettingsService } from './settingsService'; type CacheEntryLookup = { [key: string]: CacheEntry }; -type SourceLookup = { [key: TFile['path']]: CacheEntry[] }; +export type SourceCacheEntryLookupMap = { [key: TFile['path']]: CacheEntryLookup }; export interface CacheEntry { file: TFile; @@ -29,8 +29,7 @@ export interface CacheMatch extends CacheEntry { } export class CrossbowIndexingService { - private crossbowCache: CacheEntryLookup = {}; - private sourceFileLookup: SourceLookup = {}; + private crossbowCache: SourceCacheEntryLookupMap = {}; public constructor( private readonly settingsService: CrossbowSettingsService, @@ -38,12 +37,11 @@ export class CrossbowIndexingService { ) {} private addOrUpdateCacheEntry(entry: CacheEntry, source: TFile): void { - this.crossbowCache[entry.text] = entry; - this.sourceFileLookup[source.path] = this.sourceFileLookup[source.path] ?? []; - this.sourceFileLookup[source.path].push(entry); + this.crossbowCache[source.path] = this.crossbowCache[source.path] ? this.crossbowCache[source.path] : {}; + this.crossbowCache[source.path][entry.text] = entry; } - public getCache(): CacheEntryLookup { + public getCache(): SourceCacheEntryLookupMap { return this.crossbowCache; } @@ -58,15 +56,8 @@ export class CrossbowIndexingService { public clearCacheFromFile(file: TAbstractFile): void; public clearCacheFromFile(fileOrPath: TAbstractFile | string): void { const path = typeof fileOrPath === 'string' ? fileOrPath : fileOrPath.path; - this.loggingService.debugLog(`Clearing cache for file ${path}`); - - const sourceEntries = this.sourceFileLookup[path]; - - if (sourceEntries) { - sourceEntries.forEach((entry) => delete this.crossbowCache[entry.text]); - delete this.sourceFileLookup[path]; - } + delete this.crossbowCache[path]; } // 'cache' can be passed in, if this is called from an event handler which already has the cache @@ -110,6 +101,5 @@ export class CrossbowIndexingService { public clearCache(): void { this.crossbowCache = {}; - this.sourceFileLookup = {}; } } diff --git a/src/services/suggestionsService.ts b/src/services/suggestionsService.ts index 201e356..c7aaf47 100644 --- a/src/services/suggestionsService.ts +++ b/src/services/suggestionsService.ts @@ -26,67 +26,70 @@ export class CrossbowSuggestionsService { if (!wordLookup) return []; const result: Suggestion[] = []; - const cache = this.indexingService.getCache(); + const fileCacheEntryLookupMap = this.indexingService.getCache(); Object.entries(wordLookup).forEach((entry) => { const [word, editorPositions] = entry; const matchSet: Set = new Set(); // Find matches - Object.keys(cache).forEach((cacheKey) => { - const lowercaseWord = word.toLowerCase(); - const lowercaseCacheKey = cacheKey.toLowerCase(); + Object.values(fileCacheEntryLookupMap).forEach((cache) => { + Object.keys(cache).forEach((cacheKey) => { + const lowercaseWord = word.toLowerCase(); + const lowercaseCacheKey = cacheKey.toLowerCase(); - // If reference is in the same file, and we don't want to suggest references in the same file, skip - if (!this.settingsService.getSettings().suggestInSameFile && cache[cacheKey].file === currentFile) return; + // If reference is in the same file, and we don't want to suggest references in the same file, skip + if (!this.settingsService.getSettings().suggestInSameFile && cache[cacheKey].file === currentFile) return; - // If we have a case-sensitive exact match, we always add it, even if it does not satisfy the other filters. Say we have a chapter with a heading 'C' (eg. the programming language) - // We want to match a word 'C' in the current editor, even if it is too short or is on the ignore list. - if (cacheKey === word) { - matchSet.add({ ...cache[cacheKey], rank: '🏆' }); - return; - } + // If we have a case-sensitive exact match, we always add it, even if it does not satisfy the other filters. Say we have a chapter with a heading 'C' (eg. the programming language) + // We want to match a word 'C' in the current editor, even if it is too short or is on the ignore list. + if (cacheKey === word) { + matchSet.add({ ...cache[cacheKey], rank: '🏆' }); + return; + } - // If the word is on the ignore list, skip - if (this.settingsService.getSettings().ignoredWordsCaseSensisitve.includes(word)) return; + // If the word is on the ignore list, skip + if (this.settingsService.getSettings().ignoredWordsCaseSensisitve.includes(word)) return; - // If the word is too short, skip - if (word.length <= 3) return; + // If the word is too short, skip + if (word.length <= 3) return; - // If the cache key is too short, skip - if (cacheKey.length <= this.settingsService.getSettings().minimumSuggestionWordLength) return; + // If the cache key is too short, skip + if (cacheKey.length <= this.settingsService.getSettings().minimumSuggestionWordLength) return; - // If the word is not a substring of the key or the key is not a substring of the word, skip - if ((lowercaseCacheKey.includes(lowercaseWord) || lowercaseWord.includes(lowercaseCacheKey)) === false) return; + // If the word is not a substring of the key or the key is not a substring of the word, skip + if ((lowercaseCacheKey.includes(lowercaseWord) || lowercaseWord.includes(lowercaseCacheKey)) === false) + return; - // If the word does not start with an uppercase letter, skip - if ( - this.settingsService.getSettings().ignoreOccurrencesWhichStartWithLowercaseLetter && - cacheKey[0] === lowercaseCacheKey[0] - ) - return; + // If the word does not start with an uppercase letter, skip + if ( + this.settingsService.getSettings().ignoreOccurrencesWhichStartWithLowercaseLetter && + cacheKey[0] === lowercaseCacheKey[0] + ) + return; - // If the cache key does not start with an uppercase letter, skip - if ( - this.settingsService.getSettings().ignoreSuggestionsWhichStartWithLowercaseLetter && - word[0] === lowercaseWord[0] - ) - return; + // If the cache key does not start with an uppercase letter, skip + if ( + this.settingsService.getSettings().ignoreSuggestionsWhichStartWithLowercaseLetter && + word[0] === lowercaseWord[0] + ) + return; - // If the word is a case-insensitive exact match, add as a very good suggestion - if (lowercaseCacheKey === lowercaseWord) { - matchSet.add({ ...cache[cacheKey], rank: '🥇' }); - return; - } + // If the word is a case-insensitive exact match, add as a very good suggestion + if (lowercaseCacheKey === lowercaseWord) { + matchSet.add({ ...cache[cacheKey], rank: '🥇' }); + return; + } - // If the lengths differ too much, add as not-very-good suggestion - if ((1 / cacheKey.length) * word.length <= 0.2) { - matchSet.add({ ...cache[cacheKey], rank: '🥉' }); - return; - } + // If the lengths differ too much, add as not-very-good suggestion + if ((1 / cacheKey.length) * word.length <= 0.2) { + matchSet.add({ ...cache[cacheKey], rank: '🥉' }); + return; + } - // Else, add as a mediocre suggestion - matchSet.add({ ...cache[cacheKey], rank: '🥈' }); + // Else, add as a mediocre suggestion + matchSet.add({ ...cache[cacheKey], rank: '🥈' }); + }); }); if (matchSet.size > 0) { diff --git a/src/view/treeItem.ts b/src/view/treeItem.ts index 6a3e603..0eb9d12 100644 --- a/src/view/treeItem.ts +++ b/src/view/treeItem.ts @@ -33,6 +33,7 @@ export enum TreeItemButtonIcon { export class TreeItemLeaf extends HTMLElement { private readonly inner: HTMLDivElement; private readonly suffix: HTMLSpanElement; + private readonly subtitle: HTMLSpanElement; private readonly flair: HTMLSpanElement; protected readonly buttons: ButtonComponent[] = []; protected readonly mainWrapper: HTMLDivElement; @@ -70,6 +71,9 @@ export class TreeItemLeaf extends HTMLElement { this.suffix = this.inner.createEl('span', { cls: 'cb-tree-item-inner-suffix', }); + this.subtitle = this.inner.createEl('span', { + cls: 'cb-tree-item-inner-subtitle', + }); this.flair = this.flairWrapper.createEl('span', { cls: 'tree-item-flair', }); @@ -92,6 +96,10 @@ export class TreeItemLeaf extends HTMLElement { this.suffix.innerText = text; } + public addSubtitle(text: string): void { + this.subtitle.innerText = text; + } + public addButton( label: string, iconName: TreeItemButtonIcon, diff --git a/src/view/viewBuilder.ts b/src/view/viewBuilder.ts index c6dfa15..298f381 100644 --- a/src/view/viewBuilder.ts +++ b/src/view/viewBuilder.ts @@ -124,6 +124,7 @@ const createMatchTreeItems = ( }); matchTreeItem.addTextSuffix(match.cacheMatch.type); + matchTreeItem.addSubtitle(match.cacheMatch.file.name); return matchTreeItem; }); diff --git a/styles.css b/styles.css index 1b2962d..e9c9c63 100644 --- a/styles.css +++ b/styles.css @@ -3,12 +3,18 @@ } .cb-tree-item-inner-suffix { - /* font-style: italic; */ padding-left: 0.2rem; color: var(--text-faint); opacity: var(--icon-opacity); } +.cb-tree-item-inner-subtitle { + display: block; + color: var(--color-base-40); + opacity: var(--icon-opacity); + font-size: var(--font-smallest); +} + .cb-tree-item-button { box-shadow: none !important; /* Obsidian Button class overrides */ background-color: transparent !important; /* Obsidian Button class overrides */