next-video icon indicating copy to clipboard operation
next-video copied to clipboard

Allows assets to be saved in different locations instead of the filesystem

Open fain182 opened this issue 1 year ago • 4 comments
trafficstars

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.

fain182 avatar Aug 01 '24 15:08 fain182

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

vercel[bot] avatar Aug 01 '24 15:08 vercel[bot]

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 https://github.com/muxinc/next-video/discussions/143 https://github.com/muxinc/next-video/pull/204

luwes avatar Aug 02 '24 17:08 luwes

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.).

fain182 avatar Aug 02 '24 17:08 fain182

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?

mmcc avatar Aug 07 '24 17:08 mmcc

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)

fain182 avatar Sep 10 '24 12:09 fain182

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Sep 10 '24 15:09 codecov-commenter

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.

fain182 avatar Sep 10 '24 16:09 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.)

fain182 avatar Sep 16 '24 16:09 fain182