feat: remove close, sources should auto close themselves (#1663)

### Motiviation

the file closing logic has been a annoyance for consumers needing to
remember to close tiffs is hard, the file handle is often kept open for
very long times and not read frequently and the closing logic has now
leaked into the middleware

### Modifcation

- SourceFile now `open -> read -> close` per fetch, this behaviour is
reasonably similar in terms of raw performance unless there are 1000s of
reads
- .close() has been removed from all interfaces.


### Verification

Unit tests.

ref: blacha/cogeotiff#1446

Author: blacha

.github/workflows/push.yml

@@ -19,4 +19,5 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: linz/action-typescript@v3
+      - uses: linz/action-typescript@v3
+      - run: npm run test:e2e

package.json

@@ -8,7 +8,8 @@
     "version": "eslint lerna.json --fix",
     "bump": "lerna version --conventional-commits --no-push --sign-git-commit --sign-git-tag",
     "lint": "eslint . --quiet --fix --ignore-path .gitignore",
-    "test": "lerna run test"
+    "test": "lerna run test",
+    "test:e2e": "cd packages/__tests__ && node e2e.mjs"
   },
   "type": "module",
   "private": true,

packages/__tests__/e2e.mjs

@@ -161,23 +161,20 @@ async function testPrefix(prefix, fs) {
         const source = fsa.source(new URL('🦄.json', prefix));
         const bytes = Buffer.from(await source.fetch(0)).toString('hex');
         assert.equal(bytes, 'f09fa684');
-        await source.close();
       });
 
       it('should read a range from source', async () => {
         const source = fsa.source(new URL('🦄.json', prefix));
 
         const bytes = Buffer.from(await source.fetch(0, 2)).toString('hex');
         assert.equal(bytes, 'f09f');
-        await source.close();
       });
 
       it('should read from the end of the source', async () => {
         const source = fsa.source(new URL('🦄.json', prefix));
 
         const bytesEnd = Buffer.from(await source.fetch(-2)).toString('hex');
         assert.equal(bytesEnd, 'a684');
-        await source.close();
       });
     });
   });

packages/source-file/src/__test__/source.file.test.ts

@@ -1,6 +1,6 @@
 import assert from 'node:assert';
 import { join } from 'node:path';
-import { afterEach, beforeEach, describe, it } from 'node:test';
+import { beforeEach, describe, it } from 'node:test';
 import { fileURLToPath } from 'node:url';
 
 import { SourceFile } from '../index.js';
@@ -13,31 +13,16 @@ describe('SourceFile', () => {
     source = new SourceFile(TestFile);
   });
 
-  afterEach(async () => source.close());
-
-  it('should close after reads', async () => {
-    source.closeAfterRead = true;
-    assert.equal(source.fd, null);
-
-    const bytes = await source.fetch(0, 1);
-    assert.equal(bytes.byteLength, 1);
-    assert.equal(source.fd, null);
-
-    const bytesB = await source.fetch(10, 1);
-    assert.equal(bytesB.byteLength, 1);
-    assert.equal(source.fd, null);
-  });
-
   it('should resolve uri', () => {
     assert.equal(source.url.protocol, 'file:');
     assert.equal(source.url.pathname.endsWith('/source.file.test.js'), true);
   });
 
   it('should read last bytes from file', async () => {
-    const buf = Buffer.from(await source.fetch(-1024));
+    const buf = Buffer.from(await source.fetch(-128));
 
-    const metadata = source.metadata;
-    const bytes = await source.fetch((metadata?.size ?? -1) - 1024, 1024);
+    const metadata = await source.head();
+    const bytes = await source.fetch(metadata.size - 128, 128);
     const expected = Buffer.from(bytes);
 
     assert.equal(buf.toString('base64'), expected.toString('base64'));

packages/source-file/src/index.ts

@@ -4,35 +4,22 @@ import { pathToFileURL } from 'node:url';
 
 import { Source, SourceError, SourceMetadata } from '@chunkd/source';
 
+// File Stat will always return a size and lastModified
+type SourceMetadataWithSize = SourceMetadata & { size: number; lastModified: string };
+
 export class SourceFile implements Source {
   type = 'file';
-  fd?: Promise<fs.FileHandle> | null = null;
   url: URL;
 
-  /** Automatically close the file descriptor after reading */
-  closeAfterRead = false;
-
-  constructor(loc: URL | string, opts: { closeAfterRead: boolean } = { closeAfterRead: false }) {
-    this.closeAfterRead = opts.closeAfterRead;
+  constructor(loc: URL | string) {
     this.url = typeof loc === 'string' ? pathToFileURL(resolve(loc)) : loc;
   }
 
-  /** Close the file handle */
-  async close(): Promise<void> {
-    if (this.fd == null) return;
-    const fd = await this.fd;
-    if (fd == null) return;
-    await fd.close();
-    this.fd = null;
-  }
-
-  metadata?: SourceMetadata;
-  _head?: Promise<SourceMetadata>;
-  head(): Promise<SourceMetadata> {
+  metadata?: SourceMetadataWithSize;
+  _head?: Promise<SourceMetadataWithSize>;
+  head(): Promise<SourceMetadataWithSize> {
     if (this._head) return this._head;
-    if (this.fd == null) this.fd = fs.open(this.url, 'r');
-    this._head = this.fd?.then(async (fd) => {
-      const stats = await fd.stat();
+    this._head = fs.stat(this.url).then((stats) => {
       this.metadata = { size: stats.size, lastModified: stats.mtime.toString() };
       return this.metadata;
     });
@@ -49,26 +36,25 @@ export class SourceFile implements Source {
       );
     }
 
+    const metadata = await this.head();
+
     // If reading negative offsets we need the length of the file before we can read it.
     if (offset < 0) {
       length = Math.abs(offset);
-      const metadata = await this.head();
-      if (metadata.size == null) throw new SourceError(`Failed to fetch metadata from: ${this.url.href}`, 404, this);
       offset = metadata.size + offset;
-    } else if (this.metadata == null) await this.head();
-
-    const size = this.metadata?.size;
+    }
 
+    const size = metadata.size;
     // If no length given read the entire file
-    if (length == null && size != null) length = size - offset;
-
-    if (length == null || size == null) {
-      throw new SourceError(`Length is required for reading from file: ${this.url.href}`, 400, this);
+    if (length == null) length = size - offset;
+
+    // console.log({ length, offset });
+    const fd = await fs.open(this.url, 'r');
+    try {
+      const { buffer } = await fd.read(Buffer.allocUnsafe(length ?? size), 0, length, offset);
+      return buffer.buffer.slice(buffer.byteOffset, buffer.byteOffset + buffer.byteLength);
+    } finally {
+      await fd.close();
     }
-    if (this.fd == null) this.fd = fs.open(this.url, 'r');
-    const fd = await this.fd;
-    const { buffer } = await fd.read(Buffer.allocUnsafe(length ?? size), 0, length, offset);
-    if (this.closeAfterRead) await this.close();
-    return buffer.buffer.slice(buffer.byteOffset, buffer.byteOffset + buffer.byteLength);
   }
 }

packages/source/src/middleware.ts

@@ -16,7 +16,4 @@ export type SourceCallback = (req: SourceRequest) => Promise<ArrayBuffer>;
 export interface SourceMiddleware {
   name: string;
   fetch(req: SourceRequest, next: SourceCallback): Promise<ArrayBuffer>;
-
-  /** When a source is closed, this call back is fired */
-  onClose?(source: Source): Promise<void>;
 }

packages/source/src/source.ts

@@ -25,9 +25,6 @@ export interface Source {
   /** head the source to read the Metadata and sets {@link metadata} */
   head(options?: { signal: AbortSignal }): Promise<SourceMetadata>;
 
-  /** close the source, sources like files sometimes have open file handles that need to be closed */
-  close?(): Promise<void>;
-
   /**
    * Directly read bytes from the source
    *

packages/source/src/view.ts

@@ -37,11 +37,6 @@ export class SourceView implements Source {
     return this.source.head(options);
   }
 
-  async close(): Promise<void> {
-    await this.source.close?.();
-    for (const middleware of this.middleware) await middleware.onClose?.(this.source);
-  }
-
   async fetch(offset: number, length?: number, options?: { signal: AbortSignal }): Promise<ArrayBuffer> {
     const middleware = this.middleware;
     if (middleware == null || middleware.length === 0) return this.source.fetch(offset, length, options);