Refactors cache to allow identical cachekeys, adds indexingService tests

This commit is contained in:
Simon Schödler 2023-05-20 21:43:08 +02:00
parent 508bb1637b
commit e82fc2ae38
6 changed files with 125 additions and 110 deletions

View file

@ -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);

View file

@ -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 = {};
}
}

View file

@ -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<CacheMatch> = 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) {

View file

@ -33,6 +33,7 @@ export enum TreeItemButtonIcon {
export class TreeItemLeaf<TData extends ITreeVisualizable> 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<TData extends ITreeVisualizable> 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<TData extends ITreeVisualizable> extends HTMLElement {
this.suffix.innerText = text;
}
public addSubtitle(text: string): void {
this.subtitle.innerText = text;
}
public addButton(
label: string,
iconName: TreeItemButtonIcon,

View file

@ -124,6 +124,7 @@ const createMatchTreeItems = (
});
matchTreeItem.addTextSuffix(match.cacheMatch.type);
matchTreeItem.addSubtitle(match.cacheMatch.file.name);
return matchTreeItem;
});

View file

@ -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 */