servcices/supabase: Tracking issues of not fixed issues at storage-api side

[Xuanwo]

Visit #2190 for more context.

This issue has been converted as a tracking issue of not fixed issues at storage-api side:

• Special characters in filename causes uploads to fail supabase/storage#133
• The supabase storage api does not parse the RANGE http header supabase/storage#322
• HEAD /object/authenticated/<bucket>/not_found should use status code 404 supabase/storage#323

We are keeping this issue to ensure that we have a valid context for contacting the Supabase development team.

[xyjixyjixyji]

Please assign this to me, it is almost done xD

[xyjixyjixyji]

Also, I found that many tests require read_with_range capabilities, but the Supabase seems do not support the http RANGE header.

Is it reasonable to add

    if !op.info().capability().read_with_range {
        return Ok(());
    }

to tests that call into the op.range_reader()?

[Xuanwo]

Also, I found that many tests require read_with_range capabilities, but the Supabase seems do not support the http RANGE header.

Supabase seems handled range header already:

https://github.com/supabase/storage-api/blob/1794d04ab0e7259d9058293e257f442015252ec3/src/storage/renderer/asset.ts#L14-L20

  getAsset(request: FastifyRequest, options: RenderOptions) {
    return this.backend.getObject(options.bucket, options.key, options.version, {
      ifModifiedSince: request.headers['if-modified-since'],
      ifNoneMatch: request.headers['if-none-match'],
      range: request.headers.range,
    })
  }

[Xuanwo]

Supabase has two different storage backend, s3 supports range, but file not. It's another issue from supabase storage side 🤣

  async getObject(
    bucketName: string,
    key: string,
    version: string | undefined
  ): Promise<ObjectResponse> {
    const file = path.resolve(this.filePath, withOptionalVersion(`${bucketName}/${key}`, version))
    const body = fs.createReadStream(file)
    const data = await fs.stat(file)
    const { cacheControl, contentType } = await this.getFileMetadata(file)
    const lastModified = new Date(0)
    lastModified.setUTCMilliseconds(data.mtimeMs)

    const checksum = await fileChecksum(file)

    return {
      metadata: {
        cacheControl: cacheControl || 'no-cache',
        mimetype: contentType || 'application/octet-stream',
        lastModified: lastModified,
        // contentRange: data.ContentRange, @todo: support range requests
        httpStatusCode: 200,
        size: data.size,
        eTag: checksum,
        contentLength: data.size,
      },
      body,
    }
  }

[Xuanwo]

Also, I found that many tests require read_with_range capabilities, but the Supabase seems do not support the http RANGE header.

Is it reasonable to add

    if !op.info().capability().read_with_range {
        return Ok(());
    }

to tests that call into the op.range_reader()?

Yes, we should ignore tests for services that doesn't support range. Please make sure all services have this capability been set correctly.

[xyjixyjixyji]

Of course. I will patch that alongside my Supabase CI fix.

[xyjixyjixyji]

Supabase has two different storage backend, s3 supports range, but file not. It's another issue from supabase storage side 🤣

  async getObject(
    bucketName: string,
    key: string,
    version: string | undefined
  ): Promise<ObjectResponse> {
    const file = path.resolve(this.filePath, withOptionalVersion(`${bucketName}/${key}`, version))
    const body = fs.createReadStream(file)
    const data = await fs.stat(file)
    const { cacheControl, contentType } = await this.getFileMetadata(file)
    const lastModified = new Date(0)
    lastModified.setUTCMilliseconds(data.mtimeMs)

    const checksum = await fileChecksum(file)

    return {
      metadata: {
        cacheControl: cacheControl || 'no-cache',
        mimetype: contentType || 'application/octet-stream',
        lastModified: lastModified,
        // contentRange: data.ContentRange, @todo: support range requests
        httpStatusCode: 200,
        size: data.size,
        eTag: checksum,
        contentLength: data.size,
      },
      body,
    }
  }

What do you think, should we raise an issue there? 🤣

[Xuanwo]

What do you think, should we raise an issue there?

Yes, please raise an issue to supbase storage-api and link here. We can add read_with_range for supabase when they addressed it.

[Xuanwo]

Thanks to @Ji-Xinyou, most work at opendal side is done!

[Xuanwo]

We are going to remove supabase support, let's close.