Skip to content

Commit

Permalink
[FEATURE] Resource: Add isModified method
Browse files Browse the repository at this point in the history 
This allows to check whether the content of a resource has been
modified.

For example, if a build processor did not modify a resource, one could
omit writing it into the workspace resource collection. If a resource
has not been modified during the build process and has not been written
into the workspace, the FileSystem adapter can optimize the write
process, for example by using fs.copyFile.

Also in this change:
* Rename internal Resource parameter 'source' to 'sourceMetadata' and
  contained flag 'modified' to 'contentModified'
* FileSystem adapter: Do not stream into the same file when writing.
  Refactor related code
  • Loading branch information
@RandomByte
RandomByte committed Jan 20, 2023
1 parent 7d1a2c6 commit f6a590a
Show file tree
Hide file tree
Showing 6 changed files with 358 additions and 94 deletions.
68 changes: 44 additions & 24 deletions lib/Resource.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const fnTrue = () => true;
const fnFalse = () => false;

/**
* Resource
* Resource. UI5 Tooling specific representation of a file's content and metadata
*
* @public
* @class
Expand All @@ -22,7 +22,6 @@ class Resource {
*/

/**
* The constructor.
*
* @public
* @param {object} parameters Parameters
Expand All @@ -40,9 +39,10 @@ class Resource {
* string or stream).
* In some cases this is the most memory-efficient way to supply resource content
* @param {@ui5/project/specifications/Project} [parameters.project] Project this resource is associated with
* @param {object} [parameters.source] Experimental, internal parameter. Do not use
* @param {object} [parameters.sourceMetadata] Source metadata for UI5 Tooling internal use.
* Typically set by an adapter to store information for later retrieval.
*/
constructor({path, statInfo, buffer, string, createStream, stream, project, source}) {
constructor({path, statInfo, buffer, string, createStream, stream, project, sourceMetadata}) {
if (!path) {
throw new Error("Cannot create Resource: path parameter missing");
}
Expand All @@ -55,10 +55,13 @@ class Resource {
this._path = path;
this._name = Resource._getNameFromPath(path);

this._source = source; // Experimental, internal parameter
if (this._source) {
// Indicator for adapters like FileSystem to detect whether a resource has been changed
this._source.modified = this._source.modified || false;
this._sourceMetadata = sourceMetadata;
if (this._sourceMetadata) {
// This flag indicates whether a resource has changed from its original source.
// resource.isModified() is not sufficient, since it only reflects the modification state of the
// current instance.
// Since the sourceMetadata object is inherited to clones, it is the only correct indicator
this._sourceMetadata.contentModified = this._sourceMetadata.contentModified || false;
}
this.__project = project; // Two underscores since "_project" was widely used in UI5 Tooling 2.0

Expand Down Expand Up @@ -92,6 +95,8 @@ class Resource {

// Tracing:
this._collections = [];

this._isModified = false;
}

static _getNameFromPath(virPath) {
Expand Down Expand Up @@ -126,9 +131,14 @@ class Resource {
* @param {Buffer} buffer Buffer instance
*/
setBuffer(buffer) {
if (this._source && !this._source.modified) {
this._source.modified = true;
if (this._sourceMetadata) {
this._sourceMetadata.contentModified = true;
}
this._isModified = true;
this._setBuffer(buffer);
}

_setBuffer(buffer) {
this._createStream = null;
// if (this._stream) { // TODO this may cause strange issues
// this._stream.destroy();
Expand Down Expand Up @@ -212,9 +222,11 @@ class Resource {
callback for dynamic creation of a readable stream
*/
setStream(stream) {
if (this._source && !this._source.modified) {
this._source.modified = true;
this._isModified = true;
if (this._sourceMetadata) {
this._sourceMetadata.contentModified = true;
}

this._buffer = null;
// if (this._stream) { // TODO this may cause strange issues
// this._stream.destroy();
Expand Down Expand Up @@ -314,7 +326,7 @@ class Resource {
const options = {
path: this._path,
statInfo: clone(this._statInfo),
source: clone(this._source)
sourceMetadata: clone(this._sourceMetadata)
};

if (this._stream) {
Expand Down Expand Up @@ -369,6 +381,16 @@ class Resource {
return !!this.__project;
}

/**
* Check whether the content of this resource has been changed during its life cycle
*
* @public
* @returns {boolean} True if the resource's content has been changed
*/
isModified() {
return this._isModified;
}

/**
* Tracing: Get tree for printing out trace
*
Expand All @@ -386,8 +408,14 @@ class Resource {
return tree;
}

getSource() {
return this._source || {};
/**
* Returns source metadata if any where provided during the creation of this resource.
* Typically set by an adapter to store information for later retrieval.
*
* @returns {object|null}
*/
getSourceMetadata() {
return this._sourceMetadata || null;
}

/**
Expand Down Expand Up @@ -428,15 +456,7 @@ class Resource {
});
contentStream.on("end", () => {
const buffer = Buffer.concat(buffers);
let modified;
if (this._source) {
modified = this._source.modified;
}
this.setBuffer(buffer);
// Modified flag should be reset as the resource hasn't been modified from the outside
if (this._source) {
this._source.modified = modified;
}
this._setBuffer(buffer);
this._buffering = null;
resolve(buffer);
});
Expand Down
34 changes: 27 additions & 7 deletions lib/ResourceFacade.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import Resource from "./Resource.js";
*/
class ResourceFacade {
/**
* The constructor.
*
* @public
* @param {object} parameters Parameters
* @param {string} parameters.path Virtual path
* @param {@ui5/fs/Resource} parameters.resource Resource to cover
* @param {string} parameters.path Virtual path of the facade resource
* @param {@ui5/fs/Resource} parameters.resource Resource to conceal
*/
constructor({path, resource}) {
if (!path) {
Expand Down Expand Up @@ -221,13 +220,34 @@ class ResourceFacade {
hasProject() {
return this._resource.hasProject();
}
/**
* Check whether the content of this resource has been changed during its life cycle
*
* @public
* @returns {boolean} True if the resource's content has been changed
*/
isModified() {
return this._resource.isModified();
}

getConcealedResource() {
return this._resource;
/**
* Returns source metadata if any where provided during the creation of this resource.
* Typically set by an adapter to store information for later retrieval.
*
* @returns {object|null}
*/
getSourceMetadata() {
return this._resource.getSourceMetadata();
}

getSource() {
return this._resource.getSource();

/**
* Returns the resource concealed by this facade
*
* @returns {@ui5/fs/Resource}
*/
getConcealedResource() {
return this._resource;
}
}

Expand Down
79 changes: 53 additions & 26 deletions lib/adapters/FileSystem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {PassThrough} from "node:stream";
import AbstractAdapter from "./AbstractAdapter.js";

const READ_ONLY_MODE = 0o444;

const ADAPTER_NAME = "FileSystem";
/**
* File system resource adapter
*
Expand Down Expand Up @@ -70,8 +70,8 @@ class FileSystem extends AbstractAdapter {
project: this._project,
statInfo: stat,
path: this._virBaseDir,
source: {
adapter: "FileSystem",
sourceMetadata: {
adapter: ADAPTER_NAME,
fsPath: this._fsBasePath
},
createStream: () => {
Expand Down Expand Up @@ -106,8 +106,8 @@ class FileSystem extends AbstractAdapter {
project: this._project,
statInfo: stat,
path: virPath,
source: {
adapter: "FileSystem",
sourceMetadata: {
adapter: ADAPTER_NAME,
fsPath: fsPath
},
createStream: () => {
Expand Down Expand Up @@ -182,8 +182,8 @@ class FileSystem extends AbstractAdapter {
project: this._project,
statInfo,
path: virPath,
source: {
adapter: "FileSystem",
sourceMetadata: {
adapter: ADAPTER_NAME,
fsPath
}
};
Expand Down Expand Up @@ -237,28 +237,45 @@ class FileSystem extends AbstractAdapter {

await mkdir(dirPath, {recursive: true});

const resourceSource = resource.getSource();
if (!resourceSource.modified && resourceSource.adapter === "FileSystem" && resourceSource.fsPath) {
// fs.copyFile can be used when the resource is from FS and hasn't been modified
// In addition, nothing needs to be done when src === dest
if (resourceSource.fsPath === fsPath) {
log.silly(`Skip writing to ${fsPath} (Resource hasn't been modified)`);
} else {
log.silly(`Copying resource from ${resourceSource.fsPath} to ${fsPath}`);
await copyFile(resourceSource.fsPath, fsPath);
const sourceMetadata = resource.getSourceMetadata();
if (sourceMetadata && sourceMetadata.adapter === ADAPTER_NAME && sourceMetadata.fsPath) {
// Resource has been created by FileSystem adapter. This means it might require special handling

/* The following code covers these four conditions:
1. FS-paths not equal + Resource not modified => Shortcut: Use fs.copyFile
2. FS-paths equal + Resource not modified => Shortcut: Skip write altogether
3. FS-paths equal + Resource modified => Drain stream into buffer. Later write from buffer as usual
4. FS-paths not equal + Resource modified => No special handling. Write from stream or buffer
*/

if (sourceMetadata.fsPath !== fsPath && !sourceMetadata.contentModified) {
// Shortcut: fs.copyFile can be used when the resource hasn't been modified
log.silly(`Resource hasn't been modified. Copying resource from ${sourceMetadata.fsPath} to ${fsPath}`);
await copyFile(sourceMetadata.fsPath, fsPath);
if (readOnly) {
await chmod(fsPath, READ_ONLY_MODE);
}
}
return;
return;
} else if (sourceMetadata.fsPath === fsPath && !sourceMetadata.contentModified) {
log.silly(
`Resource hasn't been modified, target path equals source path. Skipping write to ${fsPath}`);
if (readOnly) {
await chmod(fsPath, READ_ONLY_MODE);
}
return;
} else if (sourceMetadata.fsPath === fsPath && sourceMetadata.contentModified) {
// Resource has been modified. Make sure all streams are drained to prevent
// issues caused by piping the original read-stream into a write-stream for the same path
await resource.getBuffer();
} else {/* Different paths + modifications require no special handling */}
}

log.silly(`Writing to ${fsPath}`);

return new Promise((resolve, reject) => {
await new Promise((resolve, reject) => {
let contentStream;

if ((drain || readOnly) && resourceSource.fsPath !== fsPath) {
if (drain || readOnly) {
// Stream will be drained
contentStream = resource.getStream();

Expand Down Expand Up @@ -292,16 +309,26 @@ class FileSystem extends AbstractAdapter {
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);
});

if (readOnly) {
if (sourceMetadata?.fsPath === fsPath) {
// When streaming into the same file, permissions need to be changed explicitly
await chmod(fsPath, READ_ONLY_MODE);
}

// In case of readOnly, we drained the stream and can now set a new callback
// for creating a stream from written file
// This should be identical to buffering the resource content in memory, since the written file
// can not be modified.
// We chose this approach to be more memory efficient in scenarios where readOnly is used
resource.setStream(function() {
return fs.createReadStream(fsPath);
});
}
}
}

Expand Down
10 changes: 6 additions & 4 deletions lib/adapters/Memory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ const log = logger.getLogger("resources:adapters:Memory");
import micromatch from "micromatch";
import AbstractAdapter from "./AbstractAdapter.js";

const ADAPTER_NAME = "Memory";

/**
* Virtual resource Adapter
*
Expand Down Expand Up @@ -76,8 +78,8 @@ class Memory extends AbstractAdapter {
return true;
}
},
source: {
adapter: "Memory"
sourceMetadata: {
adapter: ADAPTER_NAME
},
path: this._virBasePath.slice(0, -1)
})
Expand Down Expand Up @@ -155,8 +157,8 @@ class Memory extends AbstractAdapter {
if (!this._virDirs[segment]) {
this._virDirs[segment] = this._createResource({
project: this._project,
source: {
adapter: "Memory"
sourceMetadata: {
adapter: ADAPTER_NAME
},
statInfo: { // TODO: make closer to fs stat info
isDirectory: function() {
Expand Down
Loading

0 comments on commit f6a590a

Please sign in to comment.