Extend storage clients to handle Streams when used in node

[umesh-timalsina]

Resolving #1690 in a manageable way requires that we divide the changes needed into separate independent PRs. First, the storage clients should be able to operate with streams in Node.

Goals to be resolved in this issue:

• Extend the BlobClient to StreamBlobClient for supporting streams. This can be achieved relatively straight forward manner since streams are supported in node-fetch and superagent.

• Change StorageBackend and Clients with support for getStream and putStream. One question is should the existing getFile and putFile methods be updated or should we create separate methods to support streams?

• Update tests for StorageClients.

[brollb]

It would be better to simply update getFile and putFile to use streams. Otherwise, it will make it tempting to use the simpler method which uses buffers instead which will result in bugs like #1690 wherever it is used.

[brollb]

Also, it sounds like you are suggesting subclassing BlobClient, is that right? I suspect that it would be more appropriate to simply add methods like putFileStream and getFileStream to BlobClient. Admittedly, this discussion should happen in webgme-engine rather than here.

[umesh-timalsina]

Also, it sounds like you are suggesting subclassing BlobClient, is that right? I suspect that it would be more appropriate to simply add methods like putFileStream and getFileStream to BlobClient. Admittedly, this discussion should happen in webgme-engine rather than here.

Yeah, this needs to happen in webgme-engine. However, we can extend it here and contribute the changes back to webgme-engine (

deepforge/src/common/StreamBlobClient.js

Lines 41 to 60 in 6a1ddc9

	StreamBlobClient.prototype.putStream = async function (filename, stream) {
	if(require.isBrowser) {
	throw new Error('Streaming not supported in browser.');
	} else {
	const opts = {
	'headers' : this._getAuthHeaders() || {},
	'method' : 'POST',
	'body': stream
	};
	let response;
	try{
	response = await this.fetch(
	this.getCreateURL(filename, false), opts);
	} catch (e) {
	this.logger.error(`PutStream for webgme blob client failed with ${e}`);
	throw e;
	}
	return Object.keys((await response.json()))[0];
	}
	};

).

It would be better to simply update getFile and putFile to use streams. Otherwise, it will make it tempting to use the simpler method which uses buffers instead which will result in bugs like #1690 wherever it is used.

The thought process was to make sure that existing usage of getFile and putFile works (in this PR) and change the use of getFile and putFile to getStream and putStream in the code base where streams can be used. However, I have no strong leaning towards changing getFile and putFile to support streams.

[brollb]

Doing it in two steps works. It's good to break these into smaller PRs when possible :)