Skip to content

feat: store time as timespec #40

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 8, 2020
Merged

feat: store time as timespec #40

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78af06d
feat: store time as timespec
achingbrain Dec 23, 2019
9e79ec1
fix: enforce nsec value range
achingbrain Jan 2, 2020
66630b8
fix: updates to latest spec with optional mtime
achingbrain Jan 8, 2020
85b87bc
docs: document mtime behaviour
achingbrain Jan 8, 2020
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
45 changes: 30 additions & 15 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,40 +51,41 @@ function parseMtime (mtime) {
return undefined
}

// Javascript Date
if (mtime instanceof Date) {
const ms = mtime.getTime()
const secs = Math.floor(ms / 1000)

return {
secs: secs,
nsecs: (ms - (secs * 1000)) * 1000
}
}

// { secs, nsecs }
if (Object.prototype.hasOwnProperty.call(mtime, 'secs')) {
return {
mtime = {
secs: mtime.secs,
nsecs: mtime.nsecs
}
}

// UnixFS TimeSpec
if (Object.prototype.hasOwnProperty.call(mtime, 'EpochSeconds')) {
return {
mtime = {
secs: mtime.EpochSeconds,
nsecs: mtime.EpochNanoseconds
}
}

// process.hrtime()
if (Array.isArray(mtime)) {
return {
mtime = {
secs: mtime[0],
nsecs: mtime[1]
}
}

// Javascript Date
if (mtime instanceof Date) {
const ms = mtime.getTime()
const secs = Math.floor(ms / 1000)

mtime = {
secs: secs,
nsecs: (ms - (secs * 1000)) * 1000
}
}

/*
TODO: https://github.com/ipfs/aegir/issues/487

Expand All @@ -93,12 +94,22 @@ function parseMtime (mtime) {
const secs = mtime / BigInt(1e9)
const nsecs = mtime - (secs * BigInt(1e9))

return {
mtime = {
secs: parseInt(secs),
nsecs: parseInt(nsecs)
}
}
*/

if (!Object.prototype.hasOwnProperty.call(mtime, 'secs')) {
return undefined
}

if (mtime.nsecs < 0 || mtime.nsecs > 999999999) {
throw errcode(new Error('mtime-nsecs must be within the range [0,999999999]'), 'ERR_INVALID_MTIME_NSECS')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@achingbrain what did we decide on the range? Your thumbs up at ipfs/specs#236 (comment) seems to indicate you agreed with [1,999999999]...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is lagging a little behind the spec discussion.

That said, I think we can reject (or omit) 0 as a nanosecond value when serializing to a protobuf but accept it at this level as a nicety to the developer.

Be liberal in what you accept and conservative in what you send, etc, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, if the user passes the output of process.hrtime() as the mtime (e.g. [secs, nsecs] and it just so happens to fall exactly on the second with a 0 nanosecond fragment, it seems silly to reject the value as it creates a really subtle timing bug.

Instead we can just Do The Right Thing and omit the 0 value when writing out to a protobuf.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@achingbrain on the user-facing API side: I absolutely agree. Whenever I say "reject" I mean in the cases when a block is being decoded.

I.e. I make a distinction between "developer is constructing stuff to send to the network" and "we are decoding stuff we received from the network"

}

return mtime
}

function parseMode (mode) {
Expand Down Expand Up @@ -255,6 +266,10 @@ class Data {
EpochNanoseconds: parsed.nsecs
}

if (mtime.EpochNanoseconds === 0) {
delete mtime.EpochNanoseconds
}

if (parsed.secs === 0 && !parsed.nsecs) {
mtime = undefined
}
Expand Down
2 changes: 1 addition & 1 deletion src/unixfs.proto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ message Data {

message TimeSpec {
required int64 EpochSeconds = 1;
optional uint32 EpochNanoseconds = 2;
optional fixed32 EpochNanoseconds = 2;
}

message Metadata {
Expand Down