tweedegolf / storage-abstraction

Provides an abstraction layer for interacting with a storage; the storage can be local or in the cloud.
MIT License
110 stars 18 forks source link

How to extend adapter? #70

Closed maa-x closed 3 months ago

maa-x commented 4 months ago

I have extended AdapterLocal to add a method to generate public URLs.

However, there is no convenient way to use this adapter with the Storage class that I can see.

There's a few ways I can think of to make this work, but the easiest might simply be to make _adapter protected. Let me know your thoughts

abudaan commented 4 months ago

You can extend the Storage by extending the public API. But then you would have to implement the new features that you've added to the local adapter to all other adapters as well otherwise you break the abstraction layer which is the main feature and "raison d'être" of this library.

What you can do to use the functionality that you've added to AdapterLocal is:

const configuration = {
  type: StorageType.LOCAL,
  directory: "path/to/directory",
};
const storage = new Storage(configuration);
const adapter = storage.getAdapter() as AdapterLocal;
adapter.yourMethod();

// or:
(storage.adapter as AdapterLocal).yourMethod();
abudaan commented 4 months ago

Alternately you could use the adapter stand-alone:

const configuration = {
  type: StorageType.LOCAL,
  directory: "path/to/directory",
};
const adapter = new AdapterLocal();
adapter.yourMethod();

You can use the full API on an adapter instance directly. You'd only be missing switchAdapter and getAdapter.

abudaan commented 4 months ago

You can extend the protected method _getFileAsUrl in the local adapter. If you add an extra boolean option publicUrl to the options object you can switch between local urls (filepaths) and public urls.

maa-x commented 4 months ago

But then I'd lose the storage abstraction, with callers needing to handle the different storage backends.

What I was hoping was to override the getFileAsUrl method to allow it to generate a public URL, using a custom URL generator.

However, as the _adapter variable in Storage is private, it isn't possible to extend the Storage class in a way that would let me use my own AdapterLocal instead of the default one.

abudaan commented 4 months ago

You don't need to override _getFileAsUrl you can just extend it:

  protected async _getFileAsURL(
    bucketName: string,
    fileName: string,
    options: Options
  ): Promise<ResultObject> {
    // or use any other key as long as it is not used by the other adapters
    if (options.publicUrl === true) {
      try {
        // your code that generates a public url
        return { value: yourPublicUrl, error: null };
      } catch (e) {
        return { value: null, error: e.message };
      }
    } else {
      try {
        const p = path.join(this._config.directory, bucketName, fileName);
        try {
          await fs.promises.access(p);
        } catch (e) {
          return { value: null, error: e };
        }
        if (options.withoutDirectory) {
          return { value: path.join(bucketName, fileName), error: null };
        }
        return { value: p, error: null };
      } catch (e) {
        return { value: null, error: e.message };
      }
    }
  }

By using the options object you can control the way a public API method behaves in a specific adapter. Because all other adapters ignore the publicUrl property, the public API is not affected.

This is also guaranteed because the Options allows any key to be anything:

export interface Options {
  [id: string]: any; // eslint-disable-line
}
abudaan commented 4 months ago

While it is not necessary to write a complete new version of the local adapter for the functionality that you want to add, you can create custom adapters, see this part of the readme.

maa-x commented 4 months ago

I'm not a dev and have no experience with Node.js, which is probably showing. Thank you very much for taking the time to respond to me nonetheless!

Once I've extended __getFileAsUrl, how can I use it with the Storage abstraction? From what I can tell, I would need to edit the library or entirely reimplement the class. However, I believe (wrongly?) that it's best to have libraries matching their origin and not mix edited third-party libraries with the project's source.

Essentially, I was hoping I could do something like this:

var adapters = [MyLocalAdapter, S3Adapter]
const storage = Storage(storageConfigUrl, availableAdapters=adapters)

Or something along those lines, with my changes constrained to the new MyLocalAdapter, without having to use a rewritten Storage.

abudaan commented 3 months ago

Once you've extended _getFileAsUrl in your customized local adapter, you can use it in a Storage instance right away, you don't need to change anything in the Storage code. This is because when you call getFileAsUrl (without the _) on the Storage instance, your extended _getFileAsUrl (with _) will be called on the local adapter.

The Storage class is merely a shim around the adapter API. This means that you can load your custom adapter in a Storage instance as long as your adapter fully implements the adapter API.

The Storage class has only 2 extra methods: getAdapter and switchAdapter. The latter method allows you to switch to another adapter at runtime. So what you could do is:

const configLocal = {
  type: StorageType.LOCAL,
  ...
};

const configS3 = {
  type: StorageType.S3,
  ...
};

const availableAdapters = [configLocal, configS3];

const storage = new Storage(availableAdapters[0]);

// then later on in your code if you need to read/write file to S3:
storage.switchAdapter(availableAdapters[1]);

Could you share the code of your extended _getFileAsUrl method?

abudaan commented 3 months ago

I close this issue for now. Let me know if there is anything I can do to help you by reopening this issue.