feat: Allow assets to be saved in different locations

[fain182]

This proposal aims to keep next-video fully backward compatible while allowing assets to be saved to a database or sent to an API.

The idea is to make the asset-saving and uploading functions configurable, so developers can choose where to save them. The default configuration will maintain the current behavior of saving to the filesystem.

This is a draft; there are still some tests to fix, but I would appreciate any early feedback.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
next-video-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 10, 2024 2:41pm

[luwes]

thanks for the contribution! this looks great.

the question will be if we want to introduce this level of abstraction in the project, I will run this by the team.

we are thinking of adding DB support but don't have the bandwidth yet to work this through
#143
#204

[fain182]

Thanks for the quick reply!
I understand your concerns. Let me know if this could be the solution that allows developers to build their own integrations, without requiring you to allocate all the bandwidth needed to support an infinite number of different integrations (databases, APIs, etc.).

[mmcc]

This is a clever approach! I like adding the ability to do it while punting a little on what a "real/blessed" DB integration could look like. Looks like we need to update some tests, but I'm positive on this direction as a start this way.

I like leaving this PR tight and focused, but it could be nice to follow it up with an example of using it with SQLite or something?

[fain182]

Yes, that's a good idea. My first implementation will involve calling a backend to save or retrieve all assets, which could serve as an initial example.

I have restarted the implementation, focusing on keeping the tests green since I had some issues fixing them. Could you enable the CI so I can review the results? (Locally, I sometimes get inconsistent results)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.04%. Comparing base (7c0e3ab) to head (8d68562).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   90.67%   91.04%   +0.37%     
==========================================
  Files          30       30              
  Lines        2423     2445      +22     
  Branches      347      339       -8     
==========================================
+ Hits         2197     2226      +29     
+ Misses        226      219       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fain182]

Ook, the tests are green, I think you can review this...
in the next few days I will try to create a different configuration as an example.

[luwes]

(blocking) I think we want the argument here for loadAsset (and the other new hooks) to be the filePath variable because we're doing a bunch of things in getAssetConfigPath that are dependent on videoConfig settings.

Otherwise devs would have to import the getAssetConfigPath function.

[fain182]

Why devs "devs would have to import the getAssetConfigPath function" ?
The default implementation doesn't use getAssetConfigPath...

[luwes]

my bad, for some reason I was thinking someone had to provide this assetPath but it's just given to the callback.

[fain182]

This is an example of configuration that I tried and is working:

withNextVideo(config, {
  loadAsset: async function (assetPath) {
    const assetFetchResult = await fetch(`https://example.com/video-assets/${assetPath}`)

    if (!assetFetchResult.ok) {
      throw new Error('Asset not found')
    } else {
      return await assetFetchResult.json()
    }
  },
  saveAsset: async function (assetPath, asset) {
    const bodyRequest = { path: assetPath, asset: JSON.stringify(asset) }
    const assetFetchResult = await fetch(
      `https://example.com/video-assets`,
      {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
        },
        body: JSON.stringify(bodyRequest),
      }
    )

    if (!assetFetchResult.ok) {
      throw new Error(
        `Impossibile salvare video asset, status code: ${assetFetchResult.status}`
      )
    }
  },
  updateAsset: async function (assetPath, asset) {
    await this.saveAsset(assetPath, asset)
  },
  folder: 'v',
})

This is for a REST backend with a POST route /video-assets (with path and asset in the json body) to add a new videoasset or update one, and a GET route /video-assets/v/[filename] for retrieving it.
(The /v/ is because I cannot set an empy folder in the configuration.)

[luwes]

LGTM! thank you!