@ui5/fs - npm Package Compare versions

Comparing version 1.0.0 to 1.0.1

CHANGELOG.md

@@ -5,4 +5,13 @@ # Changelog
 
-A list of unreleased changes can be found [here](https://github.com/SAP/ui5-fs/compare/v1.0.0...HEAD).
+A list of unreleased changes can be found [here](https://github.com/SAP/ui5-fs/compare/v1.0.1...HEAD).
 
+<a name="v1.0.1"></a>
+## [v1.0.1] - 2019-01-31
+### Bug Fixes
+- Prevent FS write from draining Resources content [`370f121`](https://github.com/SAP/ui5-fs/commit/370f121ca4d571397c979e2dce72b6a1cf0d0005)
+
+### Dependency Updates
+- **Yarn:** Pin dir-glob dependency to v2.0.0 [`e14457c`](https://github.com/SAP/ui5-fs/commit/e14457c5b3eda1fab3d3444bca3b8406be63db2f)
+
+
 <a name="v1.0.0"></a>
@@ -34,4 +43,5 @@ ## [v1.0.0] - 2019-01-10
 
+[v1.0.1]: https://github.com/SAP/ui5-fs/compare/v1.0.0...v1.0.1
 [v1.0.0]: https://github.com/SAP/ui5-fs/compare/v0.2.0...v1.0.0
 [v0.2.0]: https://github.com/SAP/ui5-fs/compare/v0.1.0...v0.2.0
 [v0.1.0]: https://github.com/SAP/ui5-fs/compare/v0.0.1...v0.1.0

lib/AbstractReaderWriter.js

@@ -28,7 +28,18 @@ const AbstractReader = require("./AbstractReader");
 	 * @public
-	 * @param {module:@ui5/fs.Resource} resource The Resource to write
+	 * @param {module:@ui5/fs.Resource} resource Resource to write
+	 * @param {Object} [options]
+	 * @param {boolean} [options.readOnly=false] Whether the resource content shall be written read-only
+	 *						Do not use in conjunction with the <code>drain</code> option.
+	 *						The written file will be used as the new source of this resources content.
+	 *						Therefore the written file should not be altered by any means.
+	 *						Activating this option might improve overall memory consumption.
+	 * @param {boolean} [options.drain=false] Whether the resource content shall be emptied during the write process.
+	 *						Do not use in conjunction with the <code>readOnly</code> option.
+	 *						Activating this option might improve overall memory consumption.
+	 *						This should be used in cases where this is the last access to the resource.
+	 *						E.g. the final write of a resource after all processing is finished.
 	 * @returns {Promise<undefined>} Promise resolving once data has been written
 	 */
-	write(resource) {
-		return this._write(resource);
+	write(resource, options = {drain: false, readOnly: false}) {
+		return this._write(resource, options);
 	}
@@ -41,6 +52,7 @@
 	 * @protected
-	 * @param {module:@ui5/fs.Resource} resource The Resource to write
+	 * @param {module:@ui5/fs.Resource} resource Resource to write
+	 * @param {Object} [options] Write options, see above
 	 * @returns {Promise<undefined>} Promise resolving once data has been written
 	 */
-	_write(resource) {
+	_write(resource, options) {
 		throw new Error("Not implemented");
@@ -47,0 +59,0 @@ }

lib/adapters/AbstractAdapter.js

 const log = require("@ui5/logger").getLogger("resources:adapters:AbstractAdapter");
 const minimatch = require("minimatch");
 const AbstractReaderWriter = require("../AbstractReaderWriter");
+const Resource = require("../Resource");
 
@@ -52,2 +53,21 @@ /**
 			patterns = Array.prototype.concat.apply([], patterns);
+			if (!options.nodir) {
+				for (let i = patterns.length - 1; i >= 0; i--) {
+					const idx = this._virBaseDir.indexOf(patterns[i]);
+					if (patterns[i] && idx !== -1 && idx < this._virBaseDir.length) {
+						const subPath = patterns[i];
+						return Promise.resolve([
+							new Resource({
+								project: this.project,
+								statInfo: { // TODO: make closer to fs stat info
+									isDirectory: function() {
+										return true;
+									}
+								},
+								path: subPath
+							})
+						]);
+					}
+				}
+			}
 			return this._runGlob(patterns, options, trace);
@@ -66,2 +86,3 @@ });
 		return Promise.resolve().then(() => {
+			const that = this;
 			const mm = new minimatch.Minimatch(virPattern);
@@ -76,4 +97,11 @@
 					if (globPart === undefined) {
-						log.verbose("Ran out of glob parts to match. This should not happen.");
-						return -42;
+						log.verbose("Ran out of glob parts to match (this should not happen):");
+						if (that._project) { // project is optional
+							log.verbose(`Project: ${that._project.metadata.name}`);
+						}
+						log.verbose(`Virtual base path: ${that._virBaseDir}`);
+						log.verbose(`Pattern to match: ${virPattern}`);
+						log.verbose(`Current subset (tried index ${i}):`);
+						log.verbose(subset);
+						return {idx: i, virtualMatch: true};
 					}
@@ -83,3 +111,3 @@ const basePathPart = basePathParts[i];
 						if (globPart !== basePathPart) {
-							return -42;
+							return null;
 						} else {
@@ -89,6 +117,6 @@ continue;
 					} else if (globPart === minimatch.GLOBSTAR) {
-						return i;
+						return {idx: i};
 					} else { // Regex
 						if (!globPart.test(basePathPart)) {
-							return -42;
+							return null;
 						} else {
@@ -100,5 +128,5 @@ continue;
 				if (subset.length === basePathParts.length) {
-					return -1;
+					return {rootMatch: true};
 				}
-				return i;
+				return {idx: i};
 			}
@@ -108,9 +136,11 @@
 			for (let i = 0; i < mm.set.length; i++) {
-				const matchIdx = matchSubset(mm.set[i]);
-				let resultPattern;
-				if (matchIdx !== -42) {
-					if (matchIdx === -1) { // matched one up
+				const match = matchSubset(mm.set[i]);
+				if (match) {
+					let resultPattern;
+					if (match.virtualMatch) {
+						resultPattern = basePathParts.slice(0, match.idx).join("/");
+					} else if (match.rootMatch) { // matched one up
 						resultPattern = ""; // root "/"
 					} else { // matched at some part of the glob
-						resultPattern = mm.globParts[i].slice(matchIdx).join("/");
+						resultPattern = mm.globParts[i].slice(match.idx).join("/");
 						if (resultPattern.startsWith("/")) {
@@ -117,0 +147,0 @@ resultPattern = resultPattern.substr(1);

lib/adapters/FileSystem.js

@@ -6,2 +6,3 @@ const log = require("@ui5/logger").getLogger("resources:adapters:FileSystem");
 const makeDir = require("make-dir");
+const {PassThrough} = require("stream");
 const Resource = require("../Resource");
@@ -157,3 +158,3 @@ const AbstractAdapter = require("./AbstractAdapter");
 						// Add content
-						options.createStream = () => {
+						options.createStream = function() {
 							return fs.createReadStream(fsPath);
@@ -173,6 +174,22 @@ };
 	 * @private
-	 * @param {module:@ui5/fs.Resource} resource The Resource
+	 * @param {module:@ui5/fs.Resource} resource Resource to write
+	 * @param {Object} [options]
+	 * @param {boolean} [options.readOnly] Whether the resource content shall be written read-only
+	 *						Do not use in conjunction with the <code>drain</code> option.
+	 *						The written file will be used as the new source of this resources content.
+	 *						Therefore the written file should not be altered by any means.
+	 *						Activating this option might improve overall memory consumption.
+	 * @param {boolean} [options.drain] Whether the resource content shall be emptied during the write process.
+	 *						Do not use in conjunction with the <code>readOnly</code> option.
+	 *						Activating this option might improve overall memory consumption.
+	 *						This should be used in cases where this is the last access to the resource.
+	 *						E.g. the final write of a resource after all processing is finished.
 	 * @returns {Promise<undefined>} Promise resolving once data has been written
 	 */
-	_write(resource) {
+	async _write(resource, {drain, readOnly}) {
+		if (drain && readOnly) {
+			throw new Error(`Error while writing resource ${resource.getPath()}: ` +
+				"Do not use options 'drain' and 'readOnly' at the same time.");
+		}
+
 		const relPath = resource.getPath().substr(this._virBasePath.length);
@@ -184,19 +201,49 @@ const fsPath = path.join(this._fsBasePath, relPath);
 
-		return makeDir(dirPath, {
-			fs
-		}).then(() => {
-			return new Promise((resolve, reject) => {
-				const contentStream = resource.getStream();
-				contentStream.on("error", function(err) {
+		await makeDir(dirPath, {fs});
+		return new Promise((resolve, reject) => {
+			let contentStream;
+
+			if (drain || readOnly) {
+				// Stream will be drained
+				contentStream = resource.getStream();
+
+				contentStream.on("error", (err) => {
 					reject(err);
 				});
-				const write = fs.createWriteStream(fsPath);
-				write.on("error", function(err) {
+			} else {
+				// Transform stream into buffer before writing
+				contentStream = new PassThrough();
+				const buffers = [];
+				contentStream.on("error", (err) => {
 					reject(err);
 				});
-				write.on("close", function(ex) {
-					resolve();
+				contentStream.on("data", (data) => {
+					buffers.push(data);
 				});
-				contentStream.pipe(write);
+				contentStream.on("end", () => {
+					const buffer = Buffer.concat(buffers);
+					resource.setBuffer(buffer);
+				});
+				resource.getStream().pipe(contentStream);
+			}
+
+			const writeOptions = {};
+			if (readOnly) {
+				writeOptions.mode = 0o444; // read only
+			}
+
+			const write = fs.createWriteStream(fsPath, writeOptions);
+			write.on("error", (err) => {
+				reject(err);
 			});
+			write.on("close", (ex) => {
+				if (readOnly) {
+					// Create new stream from written file
+					resource.setStream(function() {
+						return fs.createReadStream(fsPath);
+					});
+				}
+				resolve();
+			});
+			contentStream.pipe(write);
 		});
@@ -203,0 +250,0 @@ }

lib/adapters/Memory.js

@@ -53,8 +53,10 @@ const log = require("@ui5/logger").getLogger("resources:adapters:Memory");
 		const filePaths = Object.keys(this._virFiles);
-		let matchedResources = micromatch(filePaths, patterns, {
+		const matchedFilePaths = micromatch(filePaths, patterns, {
 			dot: true
 		});
+		let matchedResources = matchedFilePaths.map((virPath) => {
+			return this._virFiles[virPath];
+		});
 
 		if (!options.nodir) {
-			// TODO: Add tests for all this
 			const dirPaths = Object.keys(this._virDirs);
@@ -64,8 +66,8 @@ const matchedDirs = micromatch(dirPaths, patterns, {
 			});
-			matchedResources = matchedResources.concat(matchedDirs);
+			matchedResources = matchedResources.concat(matchedDirs.map((virPath) => {
+				return this._virDirs[virPath];
+			}));
 		}
 
-		return Promise.resolve(matchedResources.map((virPath) => {
-			return this._virFiles[virPath];
-		}));
+		return Promise.resolve(matchedResources);
 	}
@@ -122,5 +124,4 @@
 			pathSegments.forEach((segment, i) => {
-				segment = "/" + segment;
-				if (i > 1) {
-					segment = pathSegments[i - 1] + segment;
+				if (i >= 1) {
+					segment = pathSegments[i - 1] + "/" + segment;
 				}
@@ -140,3 +141,3 @@ pathSegments[i] = segment;
 						},
-						path: segment
+						path: this._virBasePath + segment
 					});
@@ -143,0 +144,0 @@ }

lib/Resource.js

@@ -16,2 +16,10 @@ const stream = require("stream");
 	/**
+	* Function for dynamic creation of content streams
+	*
+	* @public
+	* @callback module:@ui5/fs.Resource~createStream
+	* @returns {stream.Readable} A readable stream of a resources content
+	*/
+
+	/**
 	 * The constructor.
@@ -22,3 +30,4 @@ *
 	 * @param {string} parameters.path Virtual path
-	 * @param {Object} [parameters.statInfo] File stat information
+	 * @param {fs.Stats|Object} [parameters.statInfo] File information. Instance of
+	 *					[fs.Stats]{@link https://nodejs.org/api/fs.html#fs_class_fs_stats} or similar object
 	 * @param {Buffer} [parameters.buffer] Content of this resources as a Buffer instance
@@ -30,4 +39,6 @@ *					(cannot be used in conjunction with parameters string, stream or createStream)
 	 *					(cannot be used in conjunction with parameters buffer, string or createStream)
-	 * @param {Function} [parameters.createStream] Function callback that returns a readable stream of the content
-	 *					of this resource (cannot be used in conjunction with parameters buffer, string or stream)
+	 * @param {module:@ui5/fs.Resource~createStream} [parameters.createStream] Function callback that returns a readable
+	 *					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
 	 */
@@ -80,14 +91,17 @@ constructor({path, statInfo, buffer, string, createStream, stream, project}) {
 	 * @public
-	 * @returns {Promise<Buffer>} A Promise resolving with a buffer of the resource content.
+	 * @returns {Promise<Buffer>} Promise resolving with a buffer of the resource content.
 	 */
-	getBuffer() {
-		return new Promise((resolve, reject) => {
-			if (this._buffer) {
-				resolve(this._buffer);
-			} else if (this._createStream || this._stream) {
-				resolve(this._getBufferFromStream());
-			} else {
-				reject(new Error(`Resource ${this._path} has no 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`);
+		}
 	}
@@ -99,3 +113,3 @@
 	 * @public
-	 * @param {Buffer} buffer A buffer instance
+	 * @param {Buffer} buffer Buffer instance
 	 */
@@ -109,2 +123,4 @@ setBuffer(buffer) {
 		this._buffer = buffer;
+		this._contentDrained = false;
+		this._streamDrained = false;
 	}
@@ -116,5 +132,10 @@
 	 * @public
-	 * @returns {Promise<string>} A Promise resolving with a string of the resource content.
+	 * @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."));
+		}
 		return this.getBuffer().then((buffer) => buffer.toString());
@@ -127,3 +148,3 @@ }
 	 * @public
-	 * @param {string} string A string
+	 * @param {string} string Resource content
 	 */
@@ -137,15 +158,37 @@ setString(string) {
 	 *
+	 * Repetitive calls of this function are only possible if new content has been set in the meantime (through
+	 * [setStream]{@link module:@ui5/fs.Resource#setStream}, [setBuffer]{@link module:@ui5/fs.Resource#setBuffer}
+	 * or [setString]{@link module:@ui5/fs.Resource#setString}). This
+	 * is to prevent consumers from accessing drained streams.
+	 *
 	 * @public
-	 * @returns {stream.Readable} A readable stream for the resource content.
+	 * @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);
-			return bufferStream;
+			contentStream = bufferStream;
 		} else if (this._createStream || this._stream) {
-			return this._getStream();
-		} else {
+			contentStream = this._getStream();
+		}
+		if (!contentStream) {
 			throw new Error(`Resource ${this._path} has no content`);
 		}
+		// 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;
 	}
@@ -157,11 +200,19 @@
 	 * @public
-	 * @param {stream.Readable} stream readable stream
+	 * @param {stream.Readable|module:@ui5/fs.Resource~createStream} stream Readable stream of the resource content or
+	 														callback for dynamic creation of a readable stream
 	 */
 	setStream(stream) {
 		this._buffer = null;
-		this._createStream = null;
 		// if (this._stream) { // TODO this may cause strange issues
 		// 	this._stream.destroy();
 		// }
-		this._stream = stream;
+		if (typeof stream === "function") {
+			this._createStream = stream;
+			this._stream = null;
+		} else {
+			this._stream = stream;
+			this._createStream = null;
+		}
+		this._contentDrained = false;
+		this._streamDrained = false;
 	}
@@ -194,3 +245,4 @@
 	 * @public
-	 * @returns {fs.Stats} An object representing an fs.Stats instance
+	 * @returns {fs.Stats|Object} Instance of [fs.Stats]{@link https://nodejs.org/api/fs.html#fs_class_fs_stats}
+	 *								or similar object
 	 */
@@ -215,6 +267,6 @@ getStatInfo() {
 	/**
-	 * Returns a clone of the resource.
+	 * Returns a clone of the resource. The clones content is independent from that of the original resource
 	 *
 	 * @public
-	 * @returns {Promise<module:@ui5/fs.Resource>} A promise resolving the resource.
+	 * @returns {Promise<module:@ui5/fs.Resource>} Promise resolving with the clone
 	 */
@@ -250,3 +302,3 @@ clone() {
 	 *
-	 * @returns {Object}
+	 * @returns {Object} Trace tree
 	 */
@@ -269,8 +321,12 @@ getPathTree() {
 	 * @private
-	 * @returns {Function} The stream
+	 * @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;
@@ -286,3 +342,6 @@ }
 	_getBufferFromStream() {
-		return new Promise((resolve, reject) => {
+		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();
@@ -299,2 +358,3 @@ const buffers = [];
 				this.setBuffer(buffer);
+				this._buffering = null;
 				resolve(buffer);
@@ -301,0 +361,0 @@ });

package.json

 {
 	"name": "@ui5/fs",
-	"version": "1.0.0",
+	"version": "1.0.1",
 	"description": "UI5 Build and Development Tooling - File System Abstraction",
@@ -65,21 +65,21 @@ "author": "SAP SE (https://www.sap.com)",
 		"check-coverage": true,
-		"lines": 60,
-		"statements": 60,
-		"functions": 55,
-		"branches": 50,
+		"statements": 85,
+		"branches": 75,
+		"functions": 80,
+		"lines": 85,
 		"watermarks": {
-			"lines": [
-				60,
+			"statements": [
+				70,
 				90
 			],
+			"branches": [
+				70,
+				90
+			],
 			"functions": [
-				55,
+				70,
 				90
 			],
-			"branches": [
-				50,
-				70
-			],
-			"statements": [
-				60,
+			"lines": [
+				70,
 				90
@@ -114,5 +114,5 @@ ]
 		"docdash": "^1.0.2",
-		"eslint": "^5.12.0",
+		"eslint": "^5.12.1",
 		"eslint-config-google": "^0.11.0",
-		"eslint-plugin-jsdoc": "^3.15.1",
+		"eslint-plugin-jsdoc": "^4.0.1",
 		"jsdoc": "^3.5.5",
@@ -122,5 +122,8 @@ "nyc": "^13.1.0",
 		"rimraf": "^2.6.3",
-		"sinon": "^7.2.2",
+		"sinon": "^7.2.3",
 		"tap-nyan": "^1.1.0"
+	},
+	"resolutions": {
+		"dir-glob": "2.0.0"
 	}
 }

No alert changes

Improved metrics

Total package byte prevSize

74186

increased by11.37%

Lines of code

1698

increased by9.27%

No dependency changes