Closed robkspeer closed 8 years ago
Awesome, makes complete sense to me. Good catch.
@robkspeer would also be a good idea to bolt in some unit test(s) for this use case, reference:
https://github.com/Esri/offline-editor-js/blob/master/test/spec/offlineTilesAdvancedSpec.js https://github.com/Esri/offline-editor-js/blob/master/test/spec/offlineTilesAdvancedTokenSpec.js https://github.com/Esri/offline-editor-js/blob/master/test/spec/offlineTilesBasicSpec.js
For now, they are manually run tests but still better than none at all.
@andygup, during unit testing i noticed that OfflineTilesAdvanced.js
clears out any server/token info already in localStorage (https://github.com/Esri/offline-editor-js/blob/master/lib/tiles/OfflineTilesAdvanced.js#L32). Don't we want to leave that info intact when loading if a valid token has already been stored? If it's not valid or it's expired, wouldn't esri/IdentityManager
just prompt to log in again, at which point the localStorage value would just be overwritten with the new token info?
If it's not valid or it's expired, wouldn't esri/IdentityManager just prompt to log in again, at which point the localStorage value would just be overwritten with the new token info?
Yes, I agree. Let's allow IdentityManager
to do its job.
There may have been some historical issues with tokens that lead to that pattern, but I believe those issues have long ago been resolved.
@robkspeer if you get a chance can you try running npm update
from your end?
It's possible npm is having issues, here's what I've been seeing since yesterday:
npm ERR! registry error parsing json
npm ERR! registry error parsing json
npm ERR! registry error parsing json
npm ERR! publish Failed PUT 503
npm ERR! Darwin 15.4.0
npm ERR! argv "node" "/usr/local/bin/npm" "publish"
npm ERR! node v0.12.2
npm ERR! npm v2.7.5
npm ERR! Unexpected token <
npm ERR!
npm ERR! <?xml version="1.0" encoding="utf-8"?>
npm ERR! <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
npm ERR! "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
npm ERR! <html>
npm ERR! <head>
npm ERR! <title>503 backend write error</title>
npm ERR! </head>
npm ERR! <body>
npm ERR! <h1>Error 503 backend write error</h1>
npm ERR! <p>backend write error</p>
npm ERR! <h3>Guru Mediation:</h3>
npm ERR! <p>Details: cache-ord1739-ORD 1459439261 4051368177</p>
npm ERR! <hr>
npm ERR! <p>Varnish cache server</p>
npm ERR! </body>
npm ERR! </html>
@robkspeer disregard. I was able to get the update pushed.
thanks - I probably wouldn't have been able to look at this until the weekend, so glad it went well!
On Apr 1, 2016, at 8:44 AM, Andy notifications@github.com wrote:
@robkspeer disregard. I was able to get the update pushed.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub
In
OfflineTilesAdvanced
andOfflineTilesBasic
, the localStorage key to store tokens for secured tiles is alwaysoffline_id_manager
.I'd like to provide a new construction parameter in the
dbConfig
object that allows explicitly naming theoffline_id_manager
. This way, if two apps from the same domain name have different secure tiles, the token can be stored and retrieved for the two apps without overwriting the previously stored token.This is similar to the option to explicitly provide values for
DB_NAME
andDB_OBJECTSTORE_NAME
, and if it's not provided, we can default tooffline_id_manager
just as the others default ifdbConfig
is not provided.