Extension support for storage adapters

[BenoitPotty]

Note: I am quite new to the Directus world, so I might not have the big picture. Here under is my contribution to start the discussions, and maybe end up to a design that could lead to an implementation. Please excuse me, and correct me, if some information are wrong.

Purpose

Question about extension support for storage adapters has been coming and going without reaching a critical mass. This discussion aims to gather the question to be answered, a tour of the existing features and how to build this without leveraging the risk of breaking changes in the future.

Main topics

The following topic need to be addressed before thinking about implementation

• What does a storage adapter class look like? What is to be kept ?
• Folder support: how to handle the Directus Folder information in the Storage Adapter?
• Custom file naming: how to manage naming conflicts?
• File duplication handling: is this the responsibility of Directus Core or the Storage Adapter?
• Two way sync: what happens when files change on the external storage?

Some other ideas

• Allow Storage Adapter to store specific information in the Directus File entity
• Allow Storage per mime type

What does a storage adapter class look like?

Comments from @rijkvanzanten

Right now, the classes contain 80% unused methods that stem from the fork of the original project. I think we should clean that up before committing to using that as the implementation spec for extensions.

Here under, the current methods of the Storage abstract class and their usage.

TL;DR; The used methods are

• delete
• exists
• getBuffer
• getStat
• getStream
• put
• flatList

interface Storage {

  // 🚫 Not used. Append content to a file.
  append(_location: string, _content: Buffer | string): Promise<Response>;

  // 🚫 Not used. Copy a file to a location
  copy(_src: string, _dest: string): Promise<Response>;

  // ✅ used in `FilesService`. Delete an existing file.
  delete(_location: string): Promise<DeleteResponse>;

  // 🚫 Not used. Returns the driver.
  public driver(): unknown;

  // ✅ used in `AssetsService`. Determines if a file or folder already exists.
  exists(_location: string): Promise<ExistsResponse>;

  // 🚫 Not used. Returns the file content as a string.
  get(_location: string, _encoding?: string): Promise<ContentResponse<string>>;

  // ✅ used in `FilesService`. Returns the file contents as a Buffer.
  getBuffer(_location: string): Promise<ContentResponse<Buffer>>;

  // 🚫 Not used. Returns signed url for an existing file.
  getSignedUrl(_location: string, _options?: SignedUrlOptions): Promise<SignedUrlResponse>;

  // ✅ used in `FilesService` and `AssetsService`. Returns the file contents as a Buffer.
  getStat(_location: string): Promise<StatResponse>;

  // ✅ used in `AssetsService`. Returns the stream for the given file. **Note**: This function should also return a `Promise` so there would be a coherence as all methods would be `async`.
  getStream(_location: string, _range?: Range): NodeJS.ReadableStream;

  // 🚫 Not used. Returns url for a given key. Note this method doesn't validates the existence of file or it's visibility status.
  getUrl(_location: string): string;

  // 🚫 Not used. Move file to a new location.
  move(_src: string, _dest: string): Promise<Response>;

  // ✅ used in `AssetsService`, `FilesService` and `server`. Creates a new file. This method will create missing directories on the fly.
  put(_location: string, _content: Buffer | NodeJS.ReadableStream | string, _type?: string): Promise<Response>

  // 🚫 Not used. Prepends content to a file.
  prepend(_location: string, _content: Buffer | string): Promise<Response>;

  // ✅ used in `FilesService`. List files with a given prefix.
  flatList(_prefix?: string): AsyncIterable<FileListResponse>;
}

Folder support

Comments from @rijkvanzanten

This is way more open ended. I recall seeing discussions fly by around the local storage adapter running into performance issues with too many files (which is something that might be solved for in other ways), and discussions around mirroring folders between "directus folders" and the file system.

Currently, the information of the folder is only stored in the Database. Files are seen as flat for the Storage Adapter.

My opinion: the Directus Folder information (the three from the root) could be also passed to the Storage Adapter methods. This way it would be the responsibility of the Storage Manager to manage what to do with it.

Custom file naming

Comments from @rijkvanzanten

Right now, Directus will use the uuid of the file + the extension for the file name on disk. This is primarily done to prevent naming conflicts, as everything is stored flat. This is fine if you consider the file storage a "black box" and manage things through Directus, but as soon as you start using other DAM services (like cloudinary) this gets a little trickier to deal with. Ideally we allow for the original file name to be used, but that resurfaces the file duplicate question

My opinion: the storage should be seen as a "black box". The responsibility could be deferred to the Storage Adapter itself by passing the Directus File entity to the Storage Adapter methods. This way, it's the Storage Adapter responsibility to manage the naming, maybe depending on a Storage Adapter configuration. This would also require the Storage Adapter to be able to store some specific information in the Directus File entity for further use. See below for this topic.

File duplication handling

Comments from @rijkvanzanten

Yet another new feature that may or may not be related / necessary right now, but good to think about: Should Directus save a checksum of a file and prevent duplicates? If so, is that something that the storage adapter cares about? (I personally think this would/should happen before the file is stored through the adapter, so this might not be relevant).

My opinion: this responsibility could also be transferred to the Storage Adapter, and activated/deactivated through a configuration of the Storage Adapter. If the Storage Adapter is able to store it's own information in the Directus File entity, I think that everything is possible. The Directus File entity would remain independent of the file as managed by the Storage Adapter. See below for this topic

Two way sync

Comments from @rijkvanzanten

Especially relevant when using services like cloudinary as the "storage", as those services will also alter / move / rename the files. If/when that happens, how do we make sure that the directus_files information is in sync?

My opinion: this is a little bit trickier, but again should be the responsibility of the Storage Adapter to handle this. That would require that the Storage adapter expose some methods to get triggered of a change on the external storage, but also have the ability to update the Directus File entity's directory. There is also here some thoughts about performances when big changes happen on the external storage.

Storage specific information on the Directus File entity

My opinion: by allowing the Storage Adapter to store specific information, many responsibilities can be transferred to the Storage Adapter. This also allows a great openness. I see something like a json object in a column. An attention on performance should be made if the database does not support json queries.

Storage adapter per mime type

My opinion: it's an idea that comes through my last development with Rokka. Rokka does not support all mime types. That could be something developed inside of the FileService, but also simply transferred to the Storage Adapter itself, if it's given the ability to get references to other Storage Adapters. All the logic would remain in the Storage Adapter.

[rbaumier]

@bpy That's a great start!

What does a storage adapter class look like?

What does a storage adapter class look like?
Comments from @rijkvanzanten

Right now, the classes contain 80% unused methods that stem from the fork of the original project. I think we should clean that up before committing to using that as the implementation spec for extensions.

Here under, the current methods of the Storage abstract class and their usage.

Should we also define all possible exceptions? I see the the current drive package already has some, this list will probably grow/shrink depending on what available methods the Storage class has to have.

Two way sync

Comments from @rijkvanzanten

Especially relevant when using services like cloudinary as the "storage", as those services will also alter / move / rename the files. If/when that happens, how do we make sure that the directus_files information is in sync?

My opinion: this is a little bit trickier, but again should be the responsibility of the Storage Adapter to handle this. That would require that the Storage adapter expose some methods to get triggered of a change on the external storage, but also have the ability to update the Directus File entity's directory. There is also here some thoughts about performances when big changes happen on the external storage.

Sorry if this is a stupid question, but is it really necessary to store files/folders information in the database? Having two sources of truth for where the files/folders are located will lead to sync problems.

Storage adapter per mime type

My opinion: it's an idea that comes through my last development with Rokka. Rokka does not support all mime types. That could be something developed inside of the FileService, but also simply transferred to the Storage Adapter itself, if it's given the ability to get references to other Storage Adapters. All the logic would remain in the Storage Adapter.

That's a great idea. That way we could use picture-only hosting providers (for example) and leave other file kinds to other storage adapters.
Each storage adapter could specify what mime types it supports and be called only when a file has a targeted one. Or it could simply return a custom exception specifying that it doesn't support this file mime type, that would then be handled by the Directus Core.

[rijkvanzanten]

Should we also define all possible exceptions? I see the the current drive package already has some, this list will probably grow/shrink depending on what available methods the Storage class has to have.

That's a good idea 👍🏻 We should fine those in the "main" drive package @directus/drive, and the other drivers should throw the errors imported from there to keep things consistent.

Sorry if this is a stupid question, but is it really necessary to store files/folders information in the database? Having two sources of truth for where the files/folders are located will lead to sync problems.

Unfortunately yes. Directus stores a lot of additional information that doesn't exist on the file itself on the database (like the directus user that uploaded it/parsed metadata/etc and any custom fields the user might have created on files). Folders aren't necessarily required, as we currently only manage folder name and tree structure (which is both a thing on most storage adapters). On folders however, it raises the discussion on how folders are presented in the system and how they're mirrored on the storage adapter, which opens up a way bigger discussion 🤔

Each storage adapter could specify what mime types it supports and be called only when a file has a targeted one. Or it could simply return a custom exception specifying that it doesn't support this file mime type, that would then be handled by the Directus Core.

I think throwing an error and let the directus core handle it is the best way forwards 👍🏻

[BenoitPotty]

Should we also define all possible exceptions? I see the the current drive package already has some, this list will probably grow/shrink depending on what available methods the Storage class has to have.

Would you list them and propose some ?

Sorry if this is a stupid question, but is it really necessary to store files/folders information in the database? Having two sources of truth for where the files/folders are located will lead to sync problems.

For me it's two different concepts. Therefore, not enforcing a structure sync between the Directus folders and the storage structure would make things really easy for the Directus Core. I think that on the build-in version, Directus should continue to ship simple storage adapters like local, s3, etc. that store the files flat. And if any developer wants to do a more clever one, it's very welcome. For more complex cases, we could propose a sort of Decorator Pattern. Let's imagine I want to do an Local Storage Adapter that sync the folder structure :

export class LocalWithFolderStorageAdapter extends StorageAdapter{
  private localStorageAdapter: StorageAdapter;
  constructor(storageManager: StorageManager) {
    this.localStorageAdapter : storageManager.disk('local');
  }

  put(_location: string, _content: Buffer | NodeJS.ReadableStream | string, _type?: string): Promise<Response> {
    // Do something with the _location for deciding the folder
    [...]
    // Create the folder on the local storage
    [...]
   this.localStorageAdapter.put(_location, _content, _type);
  }
}

Still sticking to my idea that passing the directus_file and the directus_folder entities would make that event more open.

I think throwing an error and let the directus core handle it is the best way forwards 👍🏻

I would propose the same as above: injection of the StorageManager. This way, the Storage Adapter can use another StorageAdapter before deciding to send an exception. But that's totally opened. That would look like this

export class ImageOnlyStorageAdapter extends StorageAdapter{
  private fallbackStorageAdapter: StorageAdapter;
  private myStorageClient: any;
  constructor(storageManager: StorageManager) {
    this.fallbackStorageAdapter : storageManager.disk('local');
    // create the "myStorageClient"
   [...]
  }

  put(_location: string, _content: Buffer | NodeJS.ReadableStream | string, _type?: string): Promise<Response> {
    if (this.isTypeSupported(_type)) {
      const myStorageRelativeInfo = this.myStorageClient.upload(_content); //i.e.
      // Do Something with the returned info
     return Promise.resolve()
    } else {
      return this.localStorageAdapter.put(_location, _content, _type);
  }
}

Here you can also see an example where having the directus_file entity would be nice. We could update it with some specific information coming from the storage we use.

[rijkvanzanten]

@emrahnazif You came to mind as you were having some serious production issues in the large number of files combined with the storage adapters. Mind leaving your thoughts on the points above? I'm mostly curious to hear about your suggestions around folder structures and how to handle those cross storage adapters ❤️

[emrahnazif]

@rijkvanzanten Thank you very much for asking my opinion and sorry for my delayed response. Somehow I missed the notification.
I am not expert like you but I can share my thoughts as "expert" user.

First I would like to point deleting files again. Instead of find&delete, delete action must perform with absolute path.

Now files are saved as uuid.ext and transformed images are saved uuid-uuid.ext and we never now how many transformed images we have (which is not a problem for my case).
#12248 (comment)
Your suggestion here may work for this.

I personally have a huge problem with duplicated files but I do not blame directus for this. If directus can manage duplicated files (If I understand correctly) and save storage that would be great but it is very complicated problem. "User A" and "User B" can upload same files, so a file should belong to multiple users. A user may need, multiple file entity for same file. In this case how it will work. To be honest this is beyond my technical capacities.

Two way sync looks great as an idea but It is kind of dangerous. Imagine, in a case that somehow you delete a folder accidentally and db is updated accordingly. Even if you have a backup of files, we may never know who owns the file.

Sometimes, I imagine what would happen, if saving files by their hash value as primary key (I do not even know if this is technically possible).

[rijkvanzanten]

Instead of find&delete, delete action must perform with absolute path.

This is a great point. Currently, file thumbnails are stored as <uuid>__<transformation-hash>.<ext>, which in turn means we have to do a "find all files starting with <uuid> first, before we can delete them by file path. Instead, we should probably update that to put the generated assets in a separate path, using the files UUID as the folder name, so you get something like:

original-file-with-original-name.jpg
generated/
  a1ffbc1e-150f-4fc2-b94a-b66ee99cd7ff/
    b91c5785820847687b0d0e95537112a3e7290602.jpg
    8d9c8fc4636473fafa26cba1dac0c9ea16a4ac0c.jpg

That, way, on delete, we can simply delete the file based on filename_disk in directus_files, and clear out all the generated assets by deleting everything in the folder that matches the file's UUID.

I personally have a huge problem with duplicated files but I do not blame directus for this. If directus can manage duplicated files (If I understand correctly) and save storage that would be great but it is very complicated problem. "User A" and "User B" can upload same files, so a file should belong to multiple users. A user may need, multiple file entity for same file. In this case how it will work. To be honest this is beyond my technical capacities.

Fair! I don't think we should make the file hash the primary key though, as I do believe that replacing the file contents of an existing file reference is a valid use case. That being said, adding a checksum field to directus_files and adding some sort of configuration to prevent duplicate files to be uploaded sounds like a great enhancement.

Two way sync looks great as an idea but It is kind of dangerous. Imagine, in a case that somehow you delete a folder accidentally and db is updated accordingly. Even if you have a backup of files, we may never know who owns the file.

Yeah, good point. There's a lot of gotchas we have to think through with this..

[rijkvanzanten]

Note to self: A lot of work has already been done to make the storage adapters properly modular (under @directus/storage), so I'll move this under review to make sure we can set the right expectations and updates for this feature