Skip to content

Commit

Permalink
[INTERNAL] Move resource content into separate instance
Browse files Browse the repository at this point in the history 
This should make resource clone operations cheaper since the content
doesn't need to be read and cloned immediately
  • Loading branch information
@RandomByte
RandomByte committed Dec 17, 2021
1 parent 2afde49 commit ecd7bd6
Show file tree
Hide file tree
Showing 4 changed files with 287 additions and 160 deletions.
189 changes: 42 additions & 147 deletions lib/Resource.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const stream = require("stream");
const clone = require("clone");
const path = require("path");
const nodePath = require("path");
const ResourceContent = require("./ResourceContent");

const fnTrue = () => true;
const fnFalse = () => false;
Expand Down Expand Up @@ -38,16 +38,19 @@ class Resource {
* stream of the content of this resource (cannot be used in conjunction with parameters buffer,
* string or stream).
* In some cases this is the most memory-efficient way to supply resource content
* @param {module:@ui5/fs.ResourceContent} [parameters.contentInstance] Resource content instance
* @param {object} [parameters.project] Experimental, internal parameter. Do not use
*/
constructor({path, statInfo, buffer, string, createStream, stream, project}) {
constructor({path, statInfo, buffer, string, createStream, stream, contentInstance, project}) {
if (!path) {
throw new Error("Cannot create Resource: path parameter missing");
}
if (buffer && createStream || buffer && string || string && createStream || buffer && stream ||
string && stream || createStream && stream) {
if (buffer && createStream || buffer && string || buffer && stream || buffer && contentInstance ||
string && createStream || string && stream || string && contentInstance ||
stream && createStream || stream && contentInstance ||
contentInstance && createStream) {
throw new Error("Cannot create Resource: Please set only one content parameter. " +
"Buffer, string, stream or createStream");
"Buffer, string, stream, createStream or contentInstance");
}

this._path = path;
Expand All @@ -71,14 +74,14 @@ class Resource {
birthtime: new Date()
};

if (createStream) {
this._createStream = createStream;
} else if (stream) {
this._stream = stream;
} else if (buffer) {
this.setBuffer(buffer);
} else if (typeof string === "string" || string instanceof String) {
this.setString(string);
if (contentInstance) {
this._content = contentInstance;
} else {
try {
this._content = new ResourceContent({buffer, string, createStream, stream});
} catch (err) {
throw new Error(`Failed to create content for resource ${this._path}: ${err.message}`);
}
}

// Tracing:
Expand All @@ -92,17 +95,10 @@ class Resource {
* @returns {Promise<Buffer>} Promise resolving with a buffer of the resource content.
*/
async getBuffer() {
if (this._contentDrained) {
throw new Error(`Content of Resource ${this._path} has been drained. ` +
"This might be caused by requesting resource content after a content stream has been " +
"requested and no new content (e.g. a new stream) has been set.");
}
if (this._buffer) {
return this._buffer;
} else if (this._createStream || this._stream) {
return this._getBufferFromStream();
} else {
throw new Error(`Resource ${this._path} has no content`);
try {
return await this._content.getBuffer();
} catch (err) {
throw new Error(`Failed to get buffer for resource ${this._path}: ${err.message}`);
}
}

Expand All @@ -113,14 +109,7 @@ class Resource {
* @param {Buffer} buffer Buffer instance
*/
setBuffer(buffer) {
this._createStream = null;
// if (this._stream) { // TODO this may cause strange issues
// this._stream.destroy();
// }
this._stream = null;
this._buffer = buffer;
this._contentDrained = false;
this._streamDrained = false;
this._content = new ResourceContent({buffer});
}

/**
Expand All @@ -129,13 +118,12 @@ class Resource {
* @public
* @returns {Promise<string>} Promise resolving with the resource content.
*/
getString() {
if (this._contentDrained) {
return Promise.reject(new Error(`Content of Resource ${this._path} has been drained. ` +
"This might be caused by requesting resource content after a content stream has been " +
"requested and no new content (e.g. a new stream) has been set."));
async getString() {
try {
return await this._content.getString();
} catch (err) {
throw new Error(`Failed to get string for resource ${this._path}: ${err.message}`);
}
return this.getBuffer().then((buffer) => buffer.toString());
}

/**
Expand All @@ -145,7 +133,7 @@ class Resource {
* @param {string} string Resource content
*/
setString(string) {
this.setBuffer(Buffer.from(string, "utf8"));
this._content = new ResourceContent({string});
}

/**
Expand All @@ -160,32 +148,11 @@ class Resource {
* @returns {stream.Readable} Readable stream for the resource content.
*/
getStream() {
if (this._contentDrained) {
throw new Error(`Content of Resource ${this._path} has been drained. ` +
"This might be caused by requesting resource content after a content stream has been " +
"requested and no new content (e.g. a new stream) has been set.");
}
let contentStream;
if (this._buffer) {
const bufferStream = new stream.PassThrough();
bufferStream.end(this._buffer);
contentStream = bufferStream;
} else if (this._createStream || this._stream) {
contentStream = this._getStream();
}
if (!contentStream) {
throw new Error(`Resource ${this._path} has no content`);
try {
return this._content.getStream();
} catch (err) {
throw new Error(`Failed to get stream for resource ${this._path}: ${err.message}`);
}
// If a stream instance is being returned, it will typically get drained be the consumer.
// In that case, further content access will result in a "Content stream has been drained" error.
// However, depending on the execution environment, a resources content stream might have been
// transformed into a buffer. In that case further content access is possible as a buffer can't be
// drained.
// To prevent unexpected "Content stream has been drained" errors caused by changing environments, we flag
// the resource content as "drained" every time a stream is requested. Even if actually a buffer or
// createStream callback is being used.
this._contentDrained = true;
return contentStream;
}

/**
Expand All @@ -196,19 +163,13 @@ class Resource {
callback for dynamic creation of a readable stream
*/
setStream(stream) {
this._buffer = null;
// if (this._stream) { // TODO this may cause strange issues
// this._stream.destroy();
// }
const options = {};
if (typeof stream === "function") {
this._createStream = stream;
this._stream = null;
options.createStream = stream;
} else {
this._stream = stream;
this._createStream = null;
options.stream = stream;
}
this._contentDrained = false;
this._streamDrained = false;
this._content = new ResourceContent(options);
}

/**
Expand Down Expand Up @@ -253,16 +214,11 @@ class Resource {
* @returns {Promise<number>} size in bytes, <code>0</code> if there is no content yet
*/
async getSize() {
// if resource does not have any content it should have 0 bytes
if (!this._buffer && !this._createStream && !this._stream) {
return 0;
}
const buffer = await this.getBuffer();
return buffer.byteLength;
return this._content.getSize();
}

_getNameFromPath(virPath) {
return path.posix.basename(virPath);
return nodePath.posix.basename(virPath);
}

/**
Expand All @@ -280,30 +236,14 @@ class Resource {
* @public
* @returns {Promise<module:@ui5/fs.Resource>} Promise resolving with the clone
*/
clone() {
async clone() {
const options = {
path: this._path,
statInfo: clone(this._statInfo)
};

const addContentOption = () => {
if (this._stream) {
return this._getBufferFromStream().then(function(buffer) {
options.buffer = buffer;
});
} else {
if (this._createStream) {
options.createStream = this._createStream;
} else if (this._buffer) {
options.buffer = this._buffer;
}
return Promise.resolve();
}
statInfo: clone(this._statInfo),
contentInstance: this._content
};

return addContentOption().then(() => {
return new Resource(options);
});
return new Resource(options);
}

/**
Expand All @@ -322,51 +262,6 @@ class Resource {

return tree;
}

/**
* Returns the content as stream.
*
* @private
* @returns {stream.Readable} Readable stream
*/
_getStream() {
if (this._streamDrained) {
throw new Error(`Content stream of Resource ${this._path} is flagged as drained.`);
}
if (this._createStream) {
return this._createStream();
}
this._streamDrained = true;
return this._stream;
}

/**
* Converts the buffer into a stream.
*
* @private
* @returns {Promise<Buffer>} Promise resolving with buffer.
*/
_getBufferFromStream() {
if (this._buffering) { // Prevent simultaneous buffering, causing unexpected access to drained stream
return this._buffering;
}
return this._buffering = new Promise((resolve, reject) => {
const contentStream = this._getStream();
const buffers = [];
contentStream.on("data", (data) => {
buffers.push(data);
});
contentStream.on("error", (err) => {
reject(err);
});
contentStream.on("end", () => {
const buffer = Buffer.concat(buffers);
this.setBuffer(buffer);
this._buffering = null;
resolve(buffer);
});
});
}
}

module.exports = Resource;
Loading

0 comments on commit ecd7bd6

Please sign in to comment.