Serializable #42
Serializable #42
Conversation
|
@lalitkapoor @inikulin @apsavin initial thoughts? Did I miss some way this might break existing clients? Can someone spot-check this with I'd like to polish this up with documentation, etc. if yall are good with it. |
| "looks good": function(str) { | ||
| assert.match(str, /"maxAge":null/); | ||
| "absent": function(str) { | ||
| assert.match(str, /(?!"maxAge":null)/); // NB: negative RegExp |
There was a problem hiding this comment.
Removed this since it === what's in the prototype.
| if (prop === 'expires' || | ||
| prop === 'creation' || | ||
| prop === 'lastAccessed') | ||
| { | ||
| c[prop] = obj[prop] == "Infinity" ? "Infinity" : new Date(obj[prop]); | ||
| if (obj[prop] === null) { |
There was a problem hiding this comment.
Added this check because new Date was failing, obviously. ;)
|
@stash-sfdc looks great! - left one comment above |
|
will give it a whirl with request |
|
all request tests pass. Clone the memory store jar is a frequent request in request - this is awesome. 👍 |
|
LGTM 👍 |
|
@apsavin good point. Old habits die hard so I really appreciate you pointing these out! |
This replaces the private `_creationRuntimeIdx` and `_initialCreationTime` properties with a public `.creationIndex` property. Cookies now get a creation time at construction, but this is replaced when `jar.setCookie` according to either the wall time or the `now` option to that method. Prior to this change, cookies were returned by MemoryCookieStore's getAllCookies in a strange order (sorted by domain then path then key), which breaks `cookieCompare` for cookies with creation times in the same millisecond. Sorting by `.creationIndex` and then removing that property will allow the deserializer to assign new process-specific `.creationIndex` values. But, if the underlying `Store` wants to set that upon retrieval, then they can do that too. By making the property non-enumerable, `assert.deepEqual` can work as expected ;)
|
|
||
| After a cookie has been passed through `CookieJar.setCookie()` it will have the following additional attributes: | ||
|
|
||
| * _hostOnly_ - boolean - is this a host-only cookie (i.e. no Domain field was set, but was instead implied) | ||
| * _pathIsDefault_ - boolean - if true, there was no Path field on the cookie and `defaultPath()` was used to derive one. | ||
| * _created_ - `Date` - when this cookie was added to the jar | ||
| * _creation_ - `Date` - **modified** from construction to when the cookie was added to the jar |
There was a problem hiding this comment.
Ack, this was always creation, never created :(
|
OK, revisions to retain creation sort order (see a5dfae6) and documentation are in. Is anyone going to get mad that custom properties are being dropped in |
|
I think we should follow semver, and since it's breaking let's do the major bump, followed by a release party :P |
|
Why somebody will want to use custom properties? |
|
Ok, I think we can merge now (but we need to bump version to 2.0.0 first). @stash Something else is left to do? |
|
Nothing left, I think (thanks for the nudge). I'll re-review and tag 2.0 if it's good. |
Serializable! NB: Cookie toJSON API-breaking change
|
Alright, party like it's |
|
🎉 |
|
|
|
haha, brilliant! thanks @lalitkapoor :) |
WIP until documentation is done. The branch name is misleading; I intend for this to be for all store types, not just
MemoryCookieStore(which is what I started out with).Add a
getAllCookiesinterface toCookieStoreso that stores can export all cookies. In the case of stores where this is intractable or undesirable, they can elect to not implementgetAllCookies.The serialized format is vaguely:
Importing the cookies repeatedly calls
store.putCookie(), so this should work for allCookieStoreimplementations, in theory.Cookie.fromJSONis called on each item in thecookiesArray before being passed to.putCookie().