-
Notifications
You must be signed in to change notification settings - Fork 6
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] Resource: Keep stats size up to date #253
Changes from 3 commits
31437e2
995e50f
f3c43d7
199bf06
36b912e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -74,7 +74,9 @@ class Resource { | |||||
this._stream = stream || null; | ||||||
this._buffer = buffer || null; | ||||||
if (typeof string === "string" || string instanceof String) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we rather call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||||||
this._buffer = Buffer.from(string, "utf8"); | ||||||
this.setString(string); | ||||||
} else if (this._buffer) { | ||||||
this.setBuffer(this._buffer); | ||||||
} | ||||||
|
||||||
// Tracing: | ||||||
|
@@ -117,6 +119,7 @@ class Resource { | |||||
this._buffer = buffer; | ||||||
this._contentDrained = false; | ||||||
this._streamDrained = false; | ||||||
this._setSize(Buffer.byteLength(this._buffer)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Buffer.byteLength just returns the
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
} | ||||||
|
||||||
/** | ||||||
|
@@ -207,6 +210,17 @@ class Resource { | |||||
this._streamDrained = false; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Set the size in bytes | ||||||
* | ||||||
* @param {number} size byte size | ||||||
* @private | ||||||
* @see https://nodejs.org/api/fs.html#fs_stats_size | ||||||
*/ | ||||||
_setSize(size) { | ||||||
this._statInfo.size = size; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Gets the resources path | ||||||
* | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ const test = require("ava"); | |
const Stream = require("stream"); | ||
const fs = require("fs"); | ||
const path = require("path"); | ||
const {promisify} = require("util"); | ||
const stat = promisify(fs.stat); | ||
const Resource = require("../../lib/Resource"); | ||
|
||
function createBasicResource() { | ||
|
@@ -142,6 +144,58 @@ test("Resource: setString", (t) => { | |
}); | ||
}); | ||
|
||
test("Resource: size modification", async (t) => { | ||
const resource = new Resource({ | ||
path: "my/path/to/resource" | ||
}); | ||
t.falsy(resource.getStatInfo().size, "initial size is not defined"); | ||
|
||
// string | ||
resource.setString("Content"); | ||
|
||
t.is(resource.getStatInfo().size, 7, "size after manually setting the string"); | ||
t.is(new Resource({ | ||
path: "my/path/to/resource", | ||
string: "Content" | ||
}).getStatInfo().size, 7, "size when passing string to constructor"); | ||
|
||
|
||
// buffer | ||
resource.setBuffer(Buffer.from("Super")); | ||
|
||
t.is(resource.getStatInfo().size, 5, "size after manually setting the string"); | ||
|
||
const clonedResource1 = await resource.clone(); | ||
t.is(clonedResource1.getStatInfo().size, 5, "size after cloning the resource"); | ||
|
||
|
||
// buffer with alloc | ||
const buf = Buffer.alloc(1234); | ||
buf.write("some string", 0, "utf8"); | ||
resource.setBuffer(buf); | ||
|
||
t.is(resource.getStatInfo().size, 1234, "buffer with alloc after setting the buffer"); | ||
t.is(new Resource({ | ||
path: "my/path/to/resource", | ||
buffer: buf | ||
}).getStatInfo().size, 1234, "buffer with alloc when passing buffer to constructor"); | ||
|
||
const clonedResource2 = await resource.clone(); | ||
t.is(clonedResource2.getStatInfo().size, 1234, "buffer with alloc atfer clone"); | ||
}); | ||
|
||
test("Resource: _setSize", (t) => { | ||
t.plan(1); | ||
|
||
const resource = new Resource({ | ||
path: "my/path/to/resource" | ||
}); | ||
|
||
resource._setSize(1337); | ||
|
||
t.is(resource.getStatInfo().size, 1337); | ||
}); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also add test cases when passing a real There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
test("Resource: setStream", (t) => { | ||
t.plan(1); | ||
|
||
|
@@ -267,3 +321,14 @@ test("getBuffer from Stream content: Subsequent content requests should not thro | |
// to throw "Content stream of Resource /app/index.html is flagged as drained." | ||
await t.notThrowsAsync(p2); | ||
}); | ||
|
||
test("integration stat - resource size", async (t) => { | ||
const fsPath = path.join("test", "fixtures", "application.a", "webapp", "index.html"); | ||
const statInfo = await stat(fsPath); | ||
|
||
const resource = new Resource({path: fsPath, statInfo}); | ||
t.is(resource.getStatInfo().size, 91); | ||
|
||
resource.setString("myvalue"); | ||
t.is(resource.getStatInfo().size, 7); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done