Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] adapters/Memory: Return cloned resources #235

Merged
merged 4 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ class Resource {
clone() {
const options = {
path: this._path,
statInfo: clone(this._statInfo)
statInfo: clone(this._statInfo),
project: this._project
};

const addContentOption = () => {
Expand Down
66 changes: 35 additions & 31 deletions lib/adapters/Memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,24 @@ class Memory extends AbstractAdapter {
this._virDirs = {}; // map full of directories
}

/**
* Matches and returns resources from a given map (either _virFiles or _virDirs).
*
* @private
* @param {string[]} patterns
* @param {object} resourceMap
* @returns {Promise<module:@ui5/fs.Resource[]>}
*/
async _matchPatterns(patterns, resourceMap) {
const resourcePaths = Object.keys(resourceMap);
const matchedPaths = micromatch(resourcePaths, patterns, {
dot: true
});
return Promise.all(matchedPaths.map((virPath) => {
return resourceMap[virPath] && resourceMap[virPath].clone();
}));
}

/**
* Locate resources by glob.
*
Expand All @@ -50,22 +68,11 @@ class Memory extends AbstractAdapter {
];
}

const filePaths = Object.keys(this._virFiles);
const matchedFilePaths = micromatch(filePaths, patterns, {
dot: true
});
let matchedResources = matchedFilePaths.map((virPath) => {
return this._virFiles[virPath];
});
let matchedResources = await this._matchPatterns(patterns, this._virFiles);

if (!options.nodir) {
const dirPaths = Object.keys(this._virDirs);
const matchedDirs = micromatch(dirPaths, patterns, {
dot: true
});
matchedResources = matchedResources.concat(matchedDirs.map((virPath) => {
return this._virDirs[virPath];
}));
const matchedDirs = await this._matchPatterns(patterns, this._virDirs);
matchedResources = matchedResources.concat(matchedDirs);
}

return matchedResources;
Expand All @@ -80,28 +87,25 @@ class Memory extends AbstractAdapter {
* @param {module:@ui5/fs.tracing.Trace} trace Trace instance
* @returns {Promise<module:@ui5/fs.Resource>} Promise resolving to a single resource
*/
_byPath(virPath, options, trace) {
async _byPath(virPath, options, trace) {
if (this.isPathExcluded(virPath)) {
return Promise.resolve(null);
return null;
}
if (!virPath.startsWith(this._virBasePath) && virPath !== this._virBaseDir) {
// Neither starts with basePath, nor equals baseDirectory
return null;
}
return new Promise((resolve, reject) => {
if (!virPath.startsWith(this._virBasePath) && virPath !== this._virBaseDir) {
// Neither starts with basePath, nor equals baseDirectory
resolve(null);
return;
}

const relPath = virPath.substr(this._virBasePath.length);
trace.pathCall();
const relPath = virPath.substr(this._virBasePath.length);
trace.pathCall();

const resource = this._virFiles[relPath];
const resource = this._virFiles[relPath];

if (!resource || (options.nodir && resource.getStatInfo().isDirectory())) {
resolve(null);
} else {
resolve(resource);
}
});
if (!resource || (options.nodir && resource.getStatInfo().isDirectory())) {
return null;
} else {
return await resource.clone();
}
}

/**
Expand Down
57 changes: 57 additions & 0 deletions test/lib/adapters/Memory_read.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const test = require("ava");
const {resourceFactory} = require("../../../");
const MemoryAdapter = require("../../../lib/adapters/Memory");

async function fillFromFs(readerWriter, {fsBasePath = "./test/fixtures/glob", virBasePath = "/app/"} = {}) {
const fsReader = resourceFactory.createAdapter({
Expand Down Expand Up @@ -606,3 +607,59 @@ test("static excludes: glob with negated directory exclude, not excluding resour

t.deepEqual(resources.length, 4, "Found two resources and two directories");
});

test("byPath returns new resource", async (t) => {
const originalResource = resourceFactory.createResource({
path: "/app/index.html",
string: "test"
});

const memoryAdapter = new MemoryAdapter({virBasePath: "/"});

await memoryAdapter.write(originalResource);

const returnedResource = await memoryAdapter.byPath("/app/index.html");

t.deepEqual(returnedResource, originalResource,
"Returned resource should be deep equal to original resource");
t.not(returnedResource, originalResource,
"Returned resource should not have same reference as original resource");

const anotherReturnedResource = await memoryAdapter.byPath("/app/index.html");

t.deepEqual(anotherReturnedResource, originalResource,
"Returned resource should be deep equal to original resource");
t.not(anotherReturnedResource, originalResource,
"Returned resource should not have same reference as original resource");

t.not(returnedResource, anotherReturnedResource,
"Both returned resources should not have same reference");
});

test("byGlob returns new resources", async (t) => {
const originalResource = resourceFactory.createResource({
path: "/app/index.html",
string: "test"
});

const memoryAdapter = new MemoryAdapter({virBasePath: "/"});

await memoryAdapter.write(originalResource);

const [returnedResource] = await memoryAdapter.byGlob("/**");

t.deepEqual(returnedResource, originalResource,
"Returned resource should be deep equal to the original resource");
t.not(returnedResource, originalResource,
"Returned resource should not have same reference as the original resource");

const [anotherReturnedResource] = await memoryAdapter.byGlob("/**");

t.deepEqual(anotherReturnedResource, originalResource,
"Another returned resource should be deep equal to the original resource");
t.not(anotherReturnedResource, originalResource,
"Another returned resource should not have same reference as the original resource");

t.not(returnedResource, anotherReturnedResource,
"Both returned resources should not have same reference");
});