add positioned writing feature to fs.WriteStream

[tshinnic]

While one is able to specify where writes should update a file when
using fs.write directly, if one wanted to use file streams there was
no way to set a starting position. This change implements a 'start'
option to createWriteStream() along with code to handle positioned
writing, allowing partial update of files while taking advantage of
the existing file stream handler.

A similar feature was implemented July 2010 by tuxychandru that allowed
positioned reading with createReadStream.
Support for reading byte ranges from files using fs.createReadStream July 20, 2010
f5f7cb9#lib/fs.js

That feature corresponded to the HTTP GET request with Range: header specifying
a byte range.

The feature implemented by this change would correspond to HTTP PUT requests
that specify the Content-Range: header.

This change does not implement an 'end' parameter. Unlike the situation
with ReadStream, where continued reading is controlled by the stream code,
with WriteStream the user is controlling how much to write, and may end
the writing at any point of their choice.

Files fs.js and doc/api/fs.markdown are updated and two test files added.
These may be the first tests for pwrite-style write requests?

Narrative:

Minor changes:

While examining the previous implementation for read (in order to better
conform to user expectations) a couple rough spots were seen. In more than
a couple places 'self' and 'this' were exchanged, with the usual discipline
of using 'self' only inside closures not being observed. The code was thus
a little more difficult to read. Purely out of over-fastidiousness, 'self'
was replaced with 'this' where appropriate. No change to execution, and
not strictly needed.

In createReadStream, while trying to understand the initialization code
that set 'this.pos' from 'this.start', it was realized that the extra code
in ._read() was not needed. 'this.pos' could be set initially in the
constructor. The extra 'if' could be removed from this hot code.

One lint change of double-quotes to single.

Major changes:

WriteStream.write is the correct place to queue the fs.write request with
the next file write position 'this.pos'. But where to update 'this.pos'?

In the previous code it was possible to queue an fs.write with either
Buffer or string data. Of course, a string might encode into some
not-yet-known number of bytes. While a write completion will know
exactly how many bytes were written, we can't wait until then. Multiple
writes can be queued before any writes finish, and we must update
'this.pos' before they finish. So we must discover how many bytes any
particular write will occupy as we are queueing writes.

Now since the "fs.write of string data feature" isn't supposed to be known
(undocumented), but WriteStream.write is documented to handle both Buffer
and string data, there should be little shock if encoding was done at
the stream level. Encoding from string to Buffer inside WriteStream.write
would not increase the work done, merely move the work.

The fs.write queueing code is rewritten, encoding possible strings to
Buffers, then queueing only buffers. The always available 'buffer.length'
then gives the byte count needed to update 'this.pos'.

Lastly, noticing that there were two places in the WriteStream.flush
code where fs.write was being handled, the code was re-ordered to put
fs.write handling (hot) first after error handling. (And the extra
"var self = this;" was removed)

New test file test-file-write-stream2.js is not entirely needed. It is
a variation on test-file-write-stream.js. It differs by not using some
of the undocumented callbacks that may be legacy code, but rather only
events. It also attempts to check for success by not only 'counting'
the number of events seen, but also the order of events seen. I think
this then has more information for the reader, and might catch a bug
not otherwise detectable?

New test file test-file-write-stream3.js tests the new positioned writes
with WriteStream.

[koichik]

@tshinnic - Can you divide the commit into two commits of minor changes (refactoring) and major changes (new feature)?

[tshinnic]

Two commits (I'd have to undo the one already submitted?) or two pull requests (and kill this one?) ?

I understand the desire to separate to be clear on the changes, but I'm not clear on how you want me to separate. :-)

[piscisaureus]

@tshinnic
2 commits is fine, no need to add 2 pull requests

[tshinnic]

I think it all worked eventually... two split commits resulting with the old one gone. Some day git will 'click' for me...

Each commit has its own description also, so ignore the combined description of the pull request...

Re-doing/separating got me to add a check on 'start' parameter and added that test.

[tshinnic]

I think I've responded to all the previous review comments. I've added to the description of each commit notes about the changes since previous commit.

I suppose there was a way to do this with git without losing the previous commits, but for fear of that I did copy-n-paste or made notes of each comment...

[koichik]

Why do you use 'binary' encoding?
If 'utf8' is used to write a file, it should be also used for reading.
And... I think that commit comment is tooooooooooooooooo long :-)

I did copy-n-paste

Oops. You should use git rebase -i.
example:

• Edit for ReadStream and commit (minor 2).
• Edit for WriteStream and commit (major 2).
• git rebase -i HEAD~4

Git will open up your editor:

pick ee205b1 minor corrections from examining stream read positioning
pick 2beb6c0 add positioned file writing feature to fs.WriteStream
pick 4c0e2f4 minor 2
pick a236e19 major 2

Modify as follows:

pick ee205b1 minor corrections from examining stream read positioning
fixup 4c0e2f4 minor 2
pick 2beb6c0 add positioned file writing feature to fs.WriteStream
fixup a236e19 major 2

Please try this on a temporary branch.

[tshinnic]

"And... I think that commit comment is tooooooooooooooooo long :-)"

Yes, I see that now that I see one of my other commits get into the master git log. I will rework the commits to have shorter comments.

Would it be reasonable to have a short commit comment, and then immediately add a longer comment to the pull request? Thing is, I am a stranger to y'all, and feel I should explain the changes at length...

I am thinking of starting a new page on the wiki "Contributing for Dummies". I think I've done enough wrong things to fill at least one screen worth?

[tshinnic]

Latest changes in response to review comments, to the test file in the second commit ca35782:

[Tom@TLSF15A simple]$ diff test-file-write-stream3.js.03 test-file-write-stream3.js
20,21c20
< var fileDataExpected_3 = 'abcdefghij……qrstuvwxyz';
< var fileDataExpected_4 = 'abcdefghij\u00e2\u0080\u00a6\u00e2\u0080\u00a6qrstuvwxyz';
---
> var fileDataExpected_3 = 'abcdefghij\u2026\u2026qrstuvwxyz';
64c63
<     var fileData = fs.readFileSync(filepath, 'binary');
---
>     var fileData = fs.readFileSync(filepath, 'utf8');
107c106
<     var fileData = fs.readFileSync(filepath, 'binary');
---
>     var fileData = fs.readFileSync(filepath, 'utf8');
131c130
<   var data = '\u2026\u2026',
---
>   var data = '\u2026\u2026',    // 3 bytes * 2 = 6 bytes in UTF-8
154,157d152
<     fileData = fs.readFileSync(filepath, 'binary');
<     console.log('    (debug: file data   ', fileData);
<     console.log('    (debug: expected    ', fileDataExpected_4);
<     assert.equal(fileData, fileDataExpected_4);

Done?

[koichik]

@tshinnic - Thanks, LGTM.

[koichik]

I added copyright comment to test-file-write-stream3.js.
Thanks for your contribution!