fix: use per-instance caching (#3)

To remove side effects and prevent one dns instance poisoning the
cache of another, use per-instance answer caching.

README.md

@@ -51,7 +51,8 @@ const resolver = dns({
     // will only be used to resolve `.com` addresses
     'com.': dnsJsonOverHttps('https://cloudflare-dns.com/dns-query'),
 
-    // this can also be an array, resolvers will be tried in series
+    // this can also be an array, resolvers will be shuffled and tried in
+    // series
     'net.': [
       dnsJsonOverHttps('https://dns.google/resolve'),
       dnsJsonOverHttps('https://dns.pub/dns-query')
@@ -70,18 +71,14 @@ import { dns, RecordType } from '@multiformats/dns'
 
 const resolver = dns()
 
-// resolve TXT records
+// resolve only TXT records
 const result = await dns.query('google.com', {
   types: [
     RecordType.TXT
   ]
 })
 ```
 
-N.b if multiple record types are specified, most resolvers will throw if answers
-are not available for all of them, so if you consider some record types optional
-it's better to make multiple queries.
-
 ## Caching
 
 Individual Aanswers are cached so. If you make a request, for which all

src/dns.ts

@@ -4,12 +4,17 @@ import { cache } from './utils/cache.js'
 import { getTypes } from './utils/get-types.js'
 import type { DNS as DNSInterface, DNSInit, DNSResponse, QueryOptions } from './index.js'
 import type { DNSResolver } from './resolvers/index.js'
+import type { AnswerCache } from './utils/cache.js'
+
+const DEFAULT_ANSWER_CACHE_SIZE = 1000
 
 export class DNS implements DNSInterface {
-  private resolvers: Record<string, DNSResolver[]>
+  private readonly resolvers: Record<string, DNSResolver[]>
+  private readonly cache: AnswerCache
 
   constructor (init: DNSInit) {
     this.resolvers = {}
+    this.cache = cache(init.cacheSize ?? DEFAULT_ANSWER_CACHE_SIZE)
 
     Object.entries(init.resolvers ?? {}).forEach(([tld, resolver]) => {
       if (!Array.isArray(resolver)) {
@@ -40,7 +45,7 @@ export class DNS implements DNSInterface {
    */
   async query (domain: string, options: QueryOptions = {}): Promise<DNSResponse> {
     const types = getTypes(options.types)
-    const cached = options.cached !== false ? cache.get(domain, types) : undefined
+    const cached = options.cached !== false ? this.cache.get(domain, types) : undefined
 
     if (cached != null) {
       options.onProgress?.(new CustomProgressEvent<string>('dns:cache', { detail: cached }))
@@ -68,7 +73,7 @@ export class DNS implements DNSInterface {
         })
 
         for (const answer of result.Answer) {
-          cache.add(domain, answer)
+          this.cache.add(domain, answer)
         }
 
         return result

src/index.ts

@@ -12,7 +12,7 @@
  *
  * const resolver = dns()
  *
- * // resolve A and AAAA records with a 5s timeout
+ * // resolve A records with a 5s timeout
  * const result = await dns.query('google.com', {
  *   signal: AbortSignal.timeout(5000)
  * })
@@ -217,7 +217,44 @@ export type ResolveDnsProgressEvents =
 export type DNSResolvers = Record<string, DNSResolver | DNSResolver[]>
 
 export interface DNSInit {
+  /**
+   * A set of resolvers used to answer DNS queries
+   *
+   * String keys control which resolvers are used for which TLDs.
+   *
+   * @example
+   *
+   * ```TypeScript
+   * import { dns } from '@multiformats/dns'
+   * import { dnsOverHttps } from '@multiformats/dns'
+   *
+   * const resolver = dns({
+   *   resolvers: {
+   *     // only used for .com domains
+   *     'com.': dnsOverHttps('https://example-1.com'),
+   *
+   *     // only used for .net domains, can be an array
+   *     'net.': [
+   *       dnsOverHttps('https://example-2.com'),
+   *       dnsOverHttps('https://example-3.com'),
+   *     ],
+   *
+   *     // used for everything else (can be an array)
+   *     '.': dnsOverHttps('https://example-4.com')
+   *   }
+   * })
+   * ```
+   */
   resolvers?: DNSResolvers
+
+  /**
+   * To avoid repeating DNS lookups, successful answers are cached according to
+   * their TTL. To avoid exhausting memory, this option controls how many
+   * answers to cache.
+   *
+   * @default 1000
+   */
+  cacheSize?: number
 }
 
 export function dns (init: DNSInit = {}): DNS {

src/utils/cache.ts

@@ -8,12 +8,19 @@ interface CachedAnswer {
   value: Answer
 }
 
+export interface AnswerCache {
+  get (fqdn: string, types: RecordType[]): DNSResponse | undefined
+  add (domain: string, answer: Answer): void
+  remove (domain: string, type: ResponseType): void
+  clear (): void
+}
+
 /**
  * Time Aware Least Recent Used Cache
  *
  * @see https://arxiv.org/pdf/1801.00390
  */
-export class AnswerCache {
+class CachedAnswers {
   private readonly lru: ReturnType<typeof hashlru>
 
   constructor (maxSize: number) {
@@ -92,4 +99,6 @@ export class AnswerCache {
 /**
  * Avoid sending multiple queries for the same hostname by caching results
  */
-export const cache = new AnswerCache(1000)
+export function cache (size: number): AnswerCache {
+  return new CachedAnswers(size)
+}

test/index.spec.ts

@@ -69,6 +69,37 @@ describe('dns', () => {
     expect(defaultResolver.calledOnce).to.be.true()
   })
 
+  it('should use separate caches', async () => {
+    const defaultResolver = Sinon.stub()
+
+    const answerA = {
+      name: 'another.com',
+      data: '123.123.123.123',
+      type: RecordType.A
+    }
+
+    defaultResolver.withArgs('another.com').resolves({
+      Answer: [answerA]
+    })
+
+    const resolverA = dns({
+      resolvers: {
+        '.': defaultResolver
+      }
+    })
+    const resolverB = dns({
+      resolvers: {
+        '.': defaultResolver
+      }
+    })
+    await resolverA.query('another.com')
+    await resolverA.query('another.com')
+    await resolverB.query('another.com')
+    await resolverB.query('another.com')
+
+    expect(defaultResolver.calledTwice).to.be.true()
+  })
+
   it('should ignore cache results', async () => {
     const defaultResolver = Sinon.stub()