feathersjs-ecosystem / feathers-blob

Feathers service for blob storage, like S3.
http://feathersjs.com
MIT License
92 stars 32 forks source link

Metadata does not been used on cb of createWriteStream #80

Closed dbasilioesp closed 4 years ago

dbasilioesp commented 4 years ago

Steps to reproduce

On method createWriteStream, after the uplaod of image been successfull, the callback isn´t get the metadata:

createWriteStream(opts: any, done: any) {
    let { key } = opts;
    return this.cloudinary.uploader.upload_stream({}, function (err, image) {
      done(err, { name: image.public_id });
    });
  }

Expected behavior

I expected the resolver of promise get the metada ou use for response.

claustres commented 4 years ago

Hi, what are you trying to do exactly ? createWriteStream is not part of this module, it is part of the abstract-blob-store interface so any bug related to it should be reported in the appropriate module implementing the interface.

Otherwise to use this module you should call the Feathers service interface methods instead like create.

dbasilioesp commented 4 years ago

Hi @claustres , I was trying to say that the docs of abstract-blob-store says I should return the metadata in cb, but looking in code of this module, it doesn´t look for that metadata https://github.com/feathersjs-ecosystem/feathers-blob/blob/master/lib/index.js#L104 .

I created a custom code from this module to resolve the problem I have:

async create(data: any, params?: Params): Promise<Data> {
    let { uri } = data;
    const result = parseDataURI(uri);
    const contentType = result.MIME;
    const buffer = result.buffer;

    return new Promise((resolve, reject) => {
      fromBuffer(buffer)
        .pipe(
          this.storage.createWriteStream({}, (error: any, metadata: any) => {
            if (error) {
              reject(error);
            } else {
              const res = getData(metadata);
              resolve(res);
            }
          })
        )
        .on("error", reject);
    });
  }
claustres commented 4 years ago

I am not sure to understand, this module does not implement the abstract-blob-store interface but rather the Feathers service interface. Anyway the metadata you are talking about is usually just the key identifying the object in storage (see eg https://github.com/jb55/s3-blob-store/blob/master/index.js#L91), which is also returned by the service operation here (the keyis actually the id).

I don't know what your getData method does exactly, are you missing something back from the underlying store implementation ?

dbasilioesp commented 4 years ago

Hi @claustres , the response of Cloudinary is this:

{
 "public_id": "sample_spreadsheet.xls",
 "version": 1371928603,
 "signature": "9088291a2c12202767cfa7c5e874afee72be78cd",
 "resource_type": "raw",
 "created_at": "2017-06-22T19:16:43Z",
 "tags": [],
 "bytes": 6144,
 "type": "upload",
 "etag": "107bf134b5afd11cc7544d60108d87b", 
 "url": "http://res.cloudinary.com/demo/raw/upload/v1371928603/sample_spreadsheet.xls",
 "secure_url": "https://res.cloudinary.com/demo/raw/upload/v1371928603/sample_spreadsheet.xls",
 "original_filename": "sample_spreadsheet"
}

When I return the like this from the implementation, the key isn´t been updated, and return only the id will not work for this Storage service (Cloudinary):

createWriteStream(opts: any, done: any) {
    let { key } = opts;
    return this.cloudinary.uploader.upload_stream({}, function (err, image) {
      done(err, { key: image.public_id });
    });
  }

The code I say that not look for the metadata is this: https://github.com/feathersjs-ecosystem/feathers-blob/blob/master/lib/index.js#L104

.pipe(this.Model.createWriteStream({
          key: id,
          params: params.s3
        }, (error) =>  // <============= HERE. missing the metadata
          error
            ? reject(error)
            : resolve({
              [this.id]: id,
              ...(this.returnBuffer && { buffer }),
              ...(this.returnUri && { uri }),
              size: buffer.length
            })
        ))
        .on('error', reject);
claustres commented 4 years ago

The problem is that this module is programmed according to the abstract-blob-store interface and this interface states that the key should be sufficient to perform all operations. Additional options can be provided to access more features like access rights but it should also work without.

So as far as I understand you are trying to:

1) implement a blob store targeting https://cloudinary.com/ 2) use this blob store with this module

What is missing in addition to the key property in order to make your blob store work as expected ? I guess you should identify a property that can be used as a key if possible.