feat: pluggable storage drivers (Local, S3, GCS, Azure) by khatabwedaa · Pull Request #16 · nestbolt/excel

khatabwedaa · 2026-03-25T05:25:18Z

Summary

Adds a StorageDriver interface with put(), get(), delete(), exists() methods
Implements 4 drivers: LocalDriver (default), S3Driver, GCSDriver, AzureDriver
Adds injectable DiskManager service for managing named storage backends with lazy init + caching
store(), storeFromEntity(), import(), toArray(), toCollection() now accept an optional disk parameter
Three credential strategies: SDK default chains, inline config, pre-configured client injection via forRootAsync
Zero breaking changes — existing code works identically via implicit LocalDriver

What changed

Area	Details
`src/storage/`	8 new files — interface, types, DiskManager, 4 drivers, barrels
`src/excel.service.ts`	Injected DiskManager; added `disk` param to store/import/toArray/toCollection
`src/excel.module.ts`	Added DiskManager as provider and export
`src/interfaces/excel-options.interface.ts`	Added `defaultDisk` and `disks` fields
`src/index.ts`	Re-exports for all storage types and classes
`package.json`	Optional peer deps for cloud SDKs (`@aws-sdk/client-s3`, `@google-cloud/storage`, `@azure/storage-blob`, `@azure/identity`)
`test/storage/`	6 test files — LocalDriver, S3Driver, GCSDriver, AzureDriver, DiskManager, service integration
`README.md`	Full storage driver docs with config examples for all drivers
`CHANGELOG.md`	Storage Drivers section under Unreleased

Coverage

225 tests passing (160 existing + 65 new)
99.86% statements, 99.11% branches, 100% functions, 100% lines
All new storage code at 100% coverage

Test plan

225 tests passing — no regressions
TypeScript compiles cleanly (npx tsc --noEmit)
LocalDriver tested with real temp directories (put, get, delete, exists, nested dirs, absolute paths)
S3/GCS/Azure drivers tested via client injection pattern (all operations + error handling)
DiskManager tested: implicit local, explicit configs, caching, cloud driver creation, error messages
Service integration: store/import/toArray/toCollection with disk param, DiskManager direct usage
Full backward compatibility — all 160 pre-existing tests pass unchanged

Closes #2

🤖 Generated with Claude Code

Introduces a StorageDriver interface and DiskManager so exports can be written to (and imports read from) cloud storage. Drivers are lazily instantiated and cached. Cloud SDKs are optional peer deps. store(), storeFromEntity(), import(), toArray(), and toCollection() now accept an optional `disk` parameter. Zero breaking changes — existing code works identically via the implicit LocalDriver. Closes #2 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds a pluggable storage layer to the library so exports/imports can read/write via configurable “disks” (local filesystem by default, plus S3/GCS/Azure), integrated through an injectable DiskManager and surfaced on ExcelService APIs.

Changes:

Introduces StorageDriver + DiskManager with concrete drivers: Local, S3, GCS, Azure
Extends ExcelService methods (store*, import, toArray, toCollection) to accept an optional disk parameter and wires in DiskManager
Adds extensive unit/integration tests plus docs/changelog updates and optional peer deps for cloud SDKs

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
vitest.config.ts	Updates coverage excludes for interface/type-only files and storage type/interface definitions
test/storage/service-storage.spec.ts	Adds ExcelService ↔ DiskManager integration tests for storing/reading via disks
test/storage/s3.driver.spec.ts	Adds unit tests for S3Driver behavior with mocked client and error cases
test/storage/local.driver.spec.ts	Adds unit tests for LocalDriver filesystem behavior (nested dirs, abs paths, etc.)
test/storage/gcs.driver.spec.ts	Adds unit tests for GCSDriver behavior with mocked client
test/storage/disk-manager.spec.ts	Adds unit tests for DiskManager defaulting, caching, and driver creation/errors
test/storage/azure.driver.spec.ts	Adds unit tests for AzureDriver behavior with mocked ContainerClient
test/excel.service.spec.ts	Updates ExcelService test wiring to provide DiskManager dependency
test/excel.import.spec.ts	Updates import-related tests to provide DiskManager dependency
src/storage/storage.types.ts	Defines disk configuration types for local/S3/GCS/Azure
src/storage/storage-driver.interface.ts	Defines the `StorageDriver` contract (`put/get/delete/exists`)
src/storage/index.ts	Barrel exports for storage types, DiskManager, and drivers
src/storage/drivers/s3.driver.ts	Implements S3 storage driver using AWS SDK v3 commands
src/storage/drivers/local.driver.ts	Implements filesystem-based storage driver with `root` support
src/storage/drivers/index.ts	Barrel export for driver classes
src/storage/drivers/gcs.driver.ts	Implements GCS storage driver using `@google-cloud/storage`
src/storage/drivers/azure.driver.ts	Implements Azure Blob storage driver using `@azure/storage-blob` (+ identity fallback)
src/storage/disk-manager.ts	Implements DiskManager (named disks, lazy init, caching, error messages)
src/interfaces/excel-options.interface.ts	Adds `defaultDisk` and `disks` options to module configuration
src/index.ts	Re-exports storage API from the package root
src/helpers/validate-row.ts	Adds a coverage ignore annotation for optional validator/transformer imports
src/excel.service.ts	Injects DiskManager and adds `disk` param support to storage-related methods
src/excel.module.ts	Registers and exports DiskManager from the Nest module
pnpm-lock.yaml	Adds lock entries for optional cloud SDK dev deps
package.json	Adds optional peer deps + dev deps for cloud SDKs; updates keywords
README.md	Documents storage drivers, configuration, and `disk` usage patterns
CHANGELOG.md	Adds “Storage Drivers” section under Unreleased

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T05:30:02Z

  async toCollection(
    filePath: string,
    readerType?: ExcelType,
+    disk?: string,
  ): Promise<Record<string, any>[]> {
    const type = readerType ?? this.resolveType(path.basename(filePath));
    const importable = { hasHeadingRow: true as const };
+    if (disk) {
+      const buffer = await this.diskManager.disk(disk).get(filePath);
+      const result = await readImport(importable, buffer, type, this.options);
+      return result.rows;
+    }
    const result = await readImport(importable, filePath, type, this.options);
    return result.rows;
  }


toCollection() also only uses a storage driver when disk is explicitly provided, so defaultDisk and configured local root are ignored when callers omit disk. This makes read semantics inconsistent with store() and the README (and can cause file-not-found in common setups). Consider using the resolved disk (explicit or default) and routing through DiskManager for reads.

Copilot · 2026-03-25T05:30:03Z

+      } else {
+        const { DefaultAzureCredential } = this.loadIdentitySdk();
+        const blobService = new sdk.BlobServiceClient(
+          `https://${config.accountName}.blob.core.windows.net`,
+          new DefaultAzureCredential(),
+        );
+        this.containerClient = blobService.getContainerClient(config.container);
+      }


In the DefaultAzureCredential branch, the code builds the service URL from config.accountName, but accountName is optional in AzureDiskConfig. If neither connectionString nor accountName+accountKey nor client are provided, this will produce https://undefined.blob.core.windows.net and fail in a confusing way. Consider validating that accountName is set before this branch (and throwing a clear error), or making accountName required for this auth strategy at the type level.

Copilot · 2026-03-25T05:30:03Z

+  describe("import() with disk", () => {
+    it("should import from the named disk", async () => {
+      // First store a file
+      await service.store(new SimpleExport(), "data.xlsx", undefined, "local");
+
+      // Then import it back using the disk
+      const result = await service.import({}, "data.xlsx", undefined, "local");
+      expect(result.rows.length).toBeGreaterThan(0);
+    });
+  });
+
+  describe("toArray() with disk", () => {
+    it("should read file from disk and return 2D array", async () => {
+      await service.store(new SimpleExport(), "arr.xlsx", undefined, "local");
+      const rows = await service.toArray("arr.xlsx", undefined, "local");
+      expect(rows.length).toBeGreaterThan(0);
+    });
+  });


The integration tests cover passing an explicit disk, but there’s no assertion that omitting disk uses defaultDisk (and honors a configured LocalDriver root) for read paths (import(), toArray(), toCollection()). Adding at least one test case per method without the disk argument would catch the current regression where reads fall back to raw file paths instead of the configured disk.

Copilot · 2026-03-25T05:30:03Z

  async import(
    importable: object,
    filePath: string,
    readerType?: ExcelType,
+    disk?: string,
  ): Promise<ImportResult> {
    const type = readerType ?? this.resolveType(path.basename(filePath));
+    if (disk) {
+      const buffer = await this.diskManager.disk(disk).get(filePath);
+      return readImport(importable, buffer, type, this.options);
+    }
    return readImport(importable, filePath, type, this.options);
  }


import() only reads from a storage driver when disk is explicitly passed. This ignores defaultDisk and also bypasses LocalDriver path resolution (e.g., configured root) when callers omit disk, which conflicts with the new API/docs and can break imports when defaultDisk is set. Consider resolving the effective disk name (e.g., disk ?? options.defaultDisk) and using DiskManager for reads whenever disks/defaultDisk are configured (or always), falling back to direct file-path reads only when truly using the implicit local filesystem behavior.

Copilot · 2026-03-25T05:30:03Z

  async toArray(
    filePath: string,
    readerType?: ExcelType,
+    disk?: string,
  ): Promise<any[][]> {
    const type = readerType ?? this.resolveType(path.basename(filePath));
+    if (disk) {
+      const buffer = await this.diskManager.disk(disk).get(filePath);
+      const result = await readImport({}, buffer, type, this.options);
+      return result.rows;
+    }
    const result = await readImport({}, filePath, type, this.options);
    return result.rows;
  }


toArray() has the same issue as import(): when disk is omitted it always calls readImport with a raw file path, ignoring defaultDisk and any configured local root. This will fail for non-local default disks and for local disks where paths are intended to be relative to the configured root. Consider resolving the effective disk (explicit or default) and reading via DiskManager when appropriate.

- Always route read methods (import, toArray, toCollection) through DiskManager so defaultDisk and driver config are respected consistently with store() - Add accountName validation in AzureDriver DefaultAzureCredential branch to prevent https://undefined.blob.core.windows.net URLs - Add tests for defaultDisk usage on read methods and missing accountName Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ards - LocalDriver: block relative path traversal (../) that escapes root dir - LocalDriver: switch from sync fs to fs/promises for non-blocking I/O - S3Driver/AzureDriver: guard against null response bodies in get() - Add tests for all three fixes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings March 25, 2026 05:25

Copilot started reviewing on behalf of khatabwedaa March 25, 2026 05:25 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

khatabwedaa and others added 2 commits March 25, 2026 08:36

khatabwedaa merged commit 8d0ecae into main Mar 25, 2026
2 checks passed

khatabwedaa deleted the feat/storage-drivers branch March 25, 2026 16:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: pluggable storage drivers (Local, S3, GCS, Azure)#16

feat: pluggable storage drivers (Local, S3, GCS, Azure)#16
khatabwedaa merged 3 commits into
mainfrom
feat/storage-drivers

khatabwedaa commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

khatabwedaa commented Mar 25, 2026

Summary

What changed

Coverage

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants