`context` and `metadata` Map support

オープン
  • 何か問題がありますか？
#496 コメント 3 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
45/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
停滞
技術スタック
javascript
領域
api

調査の方向性

レポートで参照されている encode_context 関数の周辺にある lib/utils/index.js から始め、続けて Map と Object の入力に対して提供されている 2 つの assertions を再現します。context と metadata が数値のキーと値を保持し、呼び出し元が事前に変換しなくても期待されるエンコード済み文字列を生成できれば完了です。

索引モデルが issue の本文から書いたものです。

説明

Bug report for Cloudinary NPM SDK

Describe the bug in a sentence or two.

The documentation states that context and metadata both support Maps when using this SDK to upload images. However, when attempting to use a Map or even a standard Object, this does not seem to be fully supported. Images are uploaded with missing or entirely omitted context and no error is thrown during the call.

Screen Shot 2021-06-15 at 5 31 12 PM

Issue Type (Can be multiple)

[ ] Build - Can’t install or import the SDK
[ ] Babel - Babel errors or cross browser issues
[ ] Performance - Performance issues
[x] Behaviour - Functions aren’t working as expected (Such as generate URL)
[x] Documentation - Inconsistency between the docs and behaviour
[ ] Incorrect Types - For typescript users who are having problems with our d.ts files
[ ] Other (Specify)

Steps to reproduce

The issue appears to be with the encode_context function which is used on both context and metadata options if provided:

https://github.com/cloudinary/cloudinary_npm/blob/4e3b0f820c3ff99aad6cccd6d3874aca95d25a50/lib/utils/index.js#L566-L571

In the case of actually providing a Map, the encode function fails to encode entirely and the result is an empty string. In the case of providing an Object, the encoding fails if any of the key/value pairs are a number. When a key value pair is a number only the value is encoded and not the key which leads to unexpected results.

The best way to test is with the following:

const { strict: assert } = require('assert')
const { encode_context: encode } = require('cloudinary/lib/utils')

const contextMap = new Map()
contextMap.set('storeNumber', 100148)
contextMap.set('vin', '5YJ3E1EA1JF045327')

// Fails: encoded context is an empty string
assert.equal(encode(contextMap), 'storeNumber=100148|vin=5YJ3E1EA1JF045327')

const contextObj = { storeNumber: 100148, vin: '5YJ3E1EA1JF045327' }

// Fails: encoded context is missing `storeNumber=`
assert.equal(encode(contextObj), 'storeNumber=100148|vin=5YJ3E1EA1JF045327')

The encoding only appears to work if an Object is supplied and all of the key/value pairs are first converted to a String.

In the meantime we've had to resort to converting our context argument to a string before passing it to the Cloudinary SDK so we ensure consistency, ex:

await cloudinary.uploader.upload(url, {
  async: true,
  context: Object.entries(context).map(val => val.join('=')).join('|')
})

Versions and Libraries (fill in the version numbers)

Cloudinary_NPM SDK version - 1.26.0
Node - 14.16.1
NPM - 6.14.13

主要言語
JavaScript
スター
672
フォーク
318
30日以内にマージされた PR はありません

環境構築

  • Dockerfile・Docker Compose ファイルなし
  • プルリクエストのテンプレートあり
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

cloudinary/cloudinary_npm のほかの issue

cloudinary/cloudinary_npm の issue をすべて見る

似ている issue

JavaScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。