From 69dd47fccd7be7180e932cdfa548bfd08d4d1645 Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Sat, 3 Apr 2021 09:27:33 +0900 Subject: Revert PrefetchAndCache --- .../completion/impl/BookmarkRepositoryImpl.ts | 25 +---- .../completion/impl/HistoryRepositoryImpl.ts | 62 ++---------- src/background/completion/impl/PrefetchAndCache.ts | 108 --------------------- src/background/completion/impl/filters.ts | 16 ++- .../completion/impl/PrefetchAndCache.test.ts | 100 ------------------- test/background/completion/impl/filters.test.ts | 30 +++++- 6 files changed, 51 insertions(+), 290 deletions(-) delete mode 100644 src/background/completion/impl/PrefetchAndCache.ts delete mode 100644 test/background/completion/impl/PrefetchAndCache.test.ts diff --git a/src/background/completion/impl/BookmarkRepositoryImpl.ts b/src/background/completion/impl/BookmarkRepositoryImpl.ts index ed6c5a6..0c95cf7 100644 --- a/src/background/completion/impl/BookmarkRepositoryImpl.ts +++ b/src/background/completion/impl/BookmarkRepositoryImpl.ts @@ -1,21 +1,9 @@ import BookmarkRepository, { BookmarkItem } from "../BookmarkRepository"; -import { HistoryItem } from "../HistoryRepository"; -import PrefetchAndCache from "./PrefetchAndCache"; const COMPLETION_ITEM_LIMIT = 10; export default class CachedBookmarkRepository implements BookmarkRepository { - private bookmarkCache: PrefetchAndCache; - - constructor() { - this.bookmarkCache = new PrefetchAndCache(this.getter, this.filter, 10); - } - - queryBookmarks(query: string): Promise { - return this.bookmarkCache.get(query); - } - - private async getter(query: string): Promise { + async queryBookmarks(query: string): Promise { const items = await browser.bookmarks.search({ query }); return items .filter((item) => item.title && item.title.length > 0) @@ -35,15 +23,4 @@ export default class CachedBookmarkRepository implements BookmarkRepository { url: item.url!, })); } - - private filter(items: HistoryItem[], query: string) { - return items.filter((item) => { - return query.split(" ").every((keyword) => { - return ( - item.title.toLowerCase().includes(keyword.toLowerCase()) || - item.url!.includes(keyword) - ); - }); - }); - } } diff --git a/src/background/completion/impl/HistoryRepositoryImpl.ts b/src/background/completion/impl/HistoryRepositoryImpl.ts index 3bf064e..789b393 100644 --- a/src/background/completion/impl/HistoryRepositoryImpl.ts +++ b/src/background/completion/impl/HistoryRepositoryImpl.ts @@ -1,49 +1,10 @@ import * as filters from "./filters"; import HistoryRepository, { HistoryItem } from "../HistoryRepository"; -import PrefetchAndCache from "./PrefetchAndCache"; const COMPLETION_ITEM_LIMIT = 10; -export default class CachedHistoryRepository implements HistoryRepository { - private historyCache: PrefetchAndCache; - - constructor() { - this.historyCache = new PrefetchAndCache(this.getter, this.filter, 10); - } - +export default class HistoryRepositoryImpl implements HistoryRepository { async queryHistories(keywords: string): Promise { - const items = await this.historyCache.get(keywords); - - const filterOrKeep = ( - source: T[], - filter: (items: T[]) => T[], - min: number - ): T[] => { - const filtered = filter(source); - if (filtered.length < min) { - return source; - } - return filtered; - }; - - return [items] - .map((items) => - filterOrKeep(items, filters.filterByPathname, COMPLETION_ITEM_LIMIT) - ) - .map((items) => - filterOrKeep(items, filters.filterByOrigin, COMPLETION_ITEM_LIMIT) - )[0] - .sort((x, y) => Number(y.visitCount) - Number(x.visitCount)) - .slice(0, COMPLETION_ITEM_LIMIT) - .map((item) => ({ - title: item.title!, - url: item.url!, - })); - } - - private async getter( - keywords: string - ): Promise { const items = await browser.history.search({ text: keywords, startTime: 0, @@ -52,17 +13,14 @@ export default class CachedHistoryRepository implements HistoryRepository { return [items] .map(filters.filterBlankTitle) .map(filters.filterHttp) - .map(filters.filterByTailingSlash)[0]; - } - - private filter(items: browser.history.HistoryItem[], query: string) { - return items.filter((item) => { - return query.split(" ").every((keyword) => { - return ( - item.title!.toLowerCase().includes(keyword.toLowerCase()) || - item.url!.includes(keyword) - ); - }); - }); + .map(filters.filterByTailingSlash) + .map((pages) => filters.filterByPathname(pages, COMPLETION_ITEM_LIMIT)) + .map((pages) => filters.filterByOrigin(pages, COMPLETION_ITEM_LIMIT))[0] + .sort((x, y) => Number(y.visitCount) - Number(x.visitCount)) + .slice(0, COMPLETION_ITEM_LIMIT) + .map((item) => ({ + title: item.title!, + url: item.url!, + })); } } diff --git a/src/background/completion/impl/PrefetchAndCache.ts b/src/background/completion/impl/PrefetchAndCache.ts deleted file mode 100644 index d2889b0..0000000 --- a/src/background/completion/impl/PrefetchAndCache.ts +++ /dev/null @@ -1,108 +0,0 @@ -type Getter = (query: string) => Promise; -type Filter = (src: T[], query: string) => T[]; - -const WHITESPACE = /\s/; - -// `shortKey` returns a shorten key to pre-fetch completions and store in the -// cache. The shorten key is generated by the following rules: -// -// 1. If the query contains a space in the middle: i.e. the query consists of -// multiple words, the method removes the last word from the query, and -// returns joined remaining words with space. -// -// 2. If the query is a single word and it's an URL, the method returns a new -// URL excluding search query with the upper path of the original URL. -// -// 3. If the query is a single word and it's not an URL, the method returns a -// word with the half-length of the original query. -// -// Examples: -// -// shortKey("hello world good bye") -// => "hello world good" -// -// shortKey("https://example.com/path/to/resource?q=hello") -// => "https://example.com/path/to/" -// -// shortKey("the-query-with-super-long-word") -// => "the-query-with-" -// -export const shortKey = (query: string): string => { - if (WHITESPACE.test(query)) { - return query - .split(WHITESPACE) - .filter((word) => word.length > 0) - .slice(0, -1) - .join(" "); - } - let url; - try { - url = new URL(query); - } catch (e) { - return query.slice(0, query.length / 2); - } - - if (url.origin === query) { - // may be on typing or removing URLs such as "such as https://goog" - return query.slice(0, query.length / 2); - } - if (url.pathname.endsWith("/")) { - // remove parameters and move to upper path - return new URL("..", url).href; - } - // remove parameters - return new URL(".", url).href; -}; - -export default class PrefetchAndCache { - private shortKey: string | undefined; - - private shortKeyCache: T[] = []; - - constructor( - private getter: Getter, - private filter: Filter, - private prefetchThrethold: number = 1 - ) {} - - async get(query: string): Promise { - query = query.trim(); - if (query.length < this.prefetchThrethold) { - this.shortKey = undefined; - return this.getter(query); - } - - if (this.needToRefresh(query)) { - this.shortKey = shortKey(query); - this.shortKeyCache = await this.getter(this.shortKey); - } - return this.filter(this.shortKeyCache, query); - } - - private needToRefresh(query: string): boolean { - if (!this.shortKey) { - // no cache - return true; - } - - if (query.length < this.shortKey.length) { - // query: "hello" - // cache: "hello_world" - return true; - } - - if (!query.startsWith(this.shortKey)) { - // queyr: "hello_w" - // shorten: "hello_morning" - return true; - } - - if (query.slice(this.shortKey.length).includes(" ")) { - // queyr: "hello x" - // shorten: "hello" - return true; - } - - return false; - } -} diff --git a/src/background/completion/impl/filters.ts b/src/background/completion/impl/filters.ts index 523491d..3d1cfb4 100644 --- a/src/background/completion/impl/filters.ts +++ b/src/background/completion/impl/filters.ts @@ -37,7 +37,7 @@ const filterByTailingSlash = (items: Item[]): Item[] => { }); }; -const filterByPathname = (items: Item[]): Item[] => { +const filterByPathname = (items: Item[], min: number): Item[] => { const hash: { [key: string]: Item } = {}; for (const item of items) { const url = new URL(item.url as string); @@ -50,10 +50,14 @@ const filterByPathname = (items: Item[]): Item[] => { hash[pathname] = item; } } - return Object.values(hash); + const filtered = Object.values(hash); + if (filtered.length < min) { + return items; + } + return filtered; }; -const filterByOrigin = (items: Item[]): Item[] => { +const filterByOrigin = (items: Item[], min: number): Item[] => { const hash: { [key: string]: Item } = {}; for (const item of items) { const origin = new URL(item.url as string).origin; @@ -65,7 +69,11 @@ const filterByOrigin = (items: Item[]): Item[] => { hash[origin] = item; } } - return Object.values(hash); + const filtered = Object.values(hash); + if (filtered.length < min) { + return items; + } + return filtered; }; export { diff --git a/test/background/completion/impl/PrefetchAndCache.test.ts b/test/background/completion/impl/PrefetchAndCache.test.ts deleted file mode 100644 index b24dfa9..0000000 --- a/test/background/completion/impl/PrefetchAndCache.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import PrefetchAndCache, { - shortKey, -} from "../../../../src/background/completion/impl/PrefetchAndCache"; -import { expect } from "chai"; - -class MockRepository { - public history: string[] = []; - - constructor(private items: string[]) {} - - get(query: string): Promise { - this.history.push(query); - if (query.length === 0) { - return Promise.resolve(this.items); - } else { - return Promise.resolve(this.items.filter((item) => item.includes(query))); - } - } -} -const filter = (items: string[], query: string) => - query.length === 0 ? items : items.filter((item) => item.includes(query)); - -describe("shortKey", () => { - it("returns query excluding the last word", () => { - const query = "hello\t world good bye"; - const shorten = shortKey(query); - expect(shorten).to.equal("hello world good"); - }); - - it("returns half-length of the query", () => { - const query = "the-query-with-super-long-word"; - const shorten = shortKey(query); - expect(shorten).to.equal("the-query-with-"); - }); - - it("returns shorten URL", () => { - let query = "https://example.com/path/to/resource?q=hello"; - let shorten = shortKey(query); - expect(shorten).to.equal("https://example.com/path/to/"); - - query = "https://example.com/path/to/resource/#id1"; - shorten = shortKey(query); - expect(shorten).to.equal("https://example.com/path/to/"); - - query = "https://www.google.c"; - shorten = shortKey(query); - expect(shorten).to.equal("https://ww"); - }); -}); - -describe("PrefetchAndCache", () => { - describe("get", () => { - it("returns cached request", async () => { - const repo = new MockRepository([ - "apple", - "apple pie", - "apple juice", - "banana", - "banana pudding", - "cherry", - ]); - const sut = new PrefetchAndCache(repo.get.bind(repo), filter); - - expect(await sut.get("apple pie")).deep.equal(["apple pie"]); - expect(await sut.get("apple ")).deep.equal([ - "apple", - "apple pie", - "apple juice", - ]); - expect(await sut.get("apple")).deep.equal([ - "apple", - "apple pie", - "apple juice", - ]); - expect(await sut.get("appl")).deep.equal([ - "apple", - "apple pie", - "apple juice", - ]); - expect(repo.history).to.deep.equal(["apple", "ap"]); - - expect(await sut.get("banana")).deep.equal(["banana", "banana pudding"]); - expect(repo.history).to.deep.equal(["apple", "ap", "ban"]); - expect(await sut.get("banana p")).deep.equal(["banana pudding"]); - expect(repo.history).to.deep.equal(["apple", "ap", "ban", "banana"]); - expect(await sut.get("ba")).deep.equal(["banana", "banana pudding"]); - expect(repo.history).to.deep.equal(["apple", "ap", "ban", "banana", "b"]); - - expect(await sut.get("")).to.have.lengthOf(6); - expect(repo.history).to.deep.equal([ - "apple", - "ap", - "ban", - "banana", - "b", - "", - ]); - }); - }); -}); diff --git a/test/background/completion/impl/filters.test.ts b/test/background/completion/impl/filters.test.ts index 70c2663..b160944 100644 --- a/test/background/completion/impl/filters.test.ts +++ b/test/background/completion/impl/filters.test.ts @@ -54,6 +54,19 @@ describe("background/usecases/filters", () => { }); describe("filterByPathname", () => { + it("remains items less than minimam length", () => { + const pages = [ + { id: "0", url: "http://i-beam.org/search?q=apple" }, + { id: "1", url: "http://i-beam.org/search?q=apple_banana" }, + { id: "2", url: "http://i-beam.org/search?q=apple_banana_cherry" }, + { id: "3", url: "http://i-beam.org/request?q=apple" }, + { id: "4", url: "http://i-beam.org/request?q=apple_banana" }, + { id: "5", url: "http://i-beam.org/request?q=apple_banana_cherry" }, + ]; + const filtered = filters.filterByPathname(pages, 10); + expect(filtered).to.have.lengthOf(6); + }); + it("filters by length of pathname", () => { const pages = [ { id: "0", url: "http://i-beam.org/search?q=apple" }, @@ -63,7 +76,7 @@ describe("background/usecases/filters", () => { { id: "4", url: "http://i-beam.net/search?q=apple_banana" }, { id: "5", url: "http://i-beam.net/search?q=apple_banana_cherry" }, ]; - const filtered = filters.filterByPathname(pages); + const filtered = filters.filterByPathname(pages, 0); expect(filtered).to.deep.equal([ { id: "0", url: "http://i-beam.org/search?q=apple" }, { id: "3", url: "http://i-beam.net/search?q=apple" }, @@ -72,6 +85,19 @@ describe("background/usecases/filters", () => { }); describe("filterByOrigin", () => { + it("remains items less than minimam length", () => { + const pages = [ + { id: "0", url: "http://i-beam.org/search?q=apple" }, + { id: "1", url: "http://i-beam.org/search?q=apple_banana" }, + { id: "2", url: "http://i-beam.org/search?q=apple_banana_cherry" }, + { id: "3", url: "http://i-beam.org/request?q=apple" }, + { id: "4", url: "http://i-beam.org/request?q=apple_banana" }, + { id: "5", url: "http://i-beam.org/request?q=apple_banana_cherry" }, + ]; + const filtered = filters.filterByOrigin(pages, 10); + expect(filtered).to.have.lengthOf(6); + }); + it("filters by length of pathname", () => { const pages = [ { id: "0", url: "http://i-beam.org/search?q=apple" }, @@ -81,7 +107,7 @@ describe("background/usecases/filters", () => { { id: "4", url: "http://i-beam.org/request?q=apple_banana" }, { id: "5", url: "http://i-beam.org/request?q=apple_banana_cherry" }, ]; - const filtered = filters.filterByOrigin(pages); + const filtered = filters.filterByOrigin(pages, 0); expect(filtered).to.deep.equal([ { id: "0", url: "http://i-beam.org/search?q=apple" }, ]); -- cgit v1.2.3