optional redirects; close #6; close #15

README.md

@@ -93,7 +93,7 @@ fs.oauthResponse(function(error, response){ });
 fs.oauthPassword(username, password, function(error, response){ });
 
 // GET
-fs.get('/platform/tree/current-person', function(error, response){ });
+fs.get('/platform/users/current', function(error, response){ });
 
 // The SDK defaults the Accept and Content-Type headers to application/x-fs-v1+json
 // for all /platform/ URLs. But that doesn't work for some endpoints which use
@@ -124,7 +124,7 @@ fs.request('/platform/tree/persons/PPPP-PPP', {
 
 // The options object is optional. When options are not include, the SDK will
 // automatically detect that the callback is the second parameter.
-// The `method` option defaults to 'GET'.
+// The `method` defaults to 'GET'.
 fs.request('/platform/tree/persons/PPPP-PPP', function(error, response){ });
 
 // Set the access token. This will also save it in a cookie if that behavior
@@ -137,6 +137,9 @@ fs.getAccessToken();
 // Delete the access token.
 fs.deleteAccessToken();
 
+// Redirects are tricky. Read more about them below.
+fs.get('/platform/tree/current-person', { followRedirect: true }, function(error, response){ })
+
 // MIDDLEWARE
 //
 // The SDK also supports middleware for modifying requests and responses. 
@@ -209,23 +212,61 @@ fs.get('/platform/tree/persons/PPPP-PPP', function(error, response){
 
 ### Redirects
 
-The [XMLHttpRequest](https://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#infrastructure-for-the-send%28%29-method)
-spec states that redirects should be transparently followed. But that leads to
-problems with the FamilySearch API because some browsers won't repeat all of the
-same options on the second request. For example, if you request JSON from
-`/platform/tree/current-person` by setting the `Accept` header to `application/x-fs-v1+json`,
-the API will responsd with a 303 redirect to the actual person URL, such as 
+__TLDR__: Automatically handling redirects is hard thus the SDK defaults to
+the platform's behavior. This may or may not be desired. Use request options
+to modify the default behavior per request.
+
+```js
+client.get('/platform/tree/current-person', {
+    expectRedirect: true,
+    followRedirect: true
+});
+```
+
+Handling redirects is tricky due to two issues. 
+
+First, browsers are configured to transparently follow 3xx redirects for 
+[XMLHttpRequest](https://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#infrastructure-for-the-send%28%29-method)
+requests. However some browsers don't repeat all of the same request options on
+the redirect. For example, if you request JSON from `/platform/tree/current-person`
+by setting the `Accept` header to `application/x-fs-v1+json`, the API will respond
+with a 303 redirect to the actual person URL, such as 
 `/platform/tree/persons/PPPP-PPP`. Then browser then will send a second request 
 to the new URL but some browsers will not copy the `Accept` header from the first
 request to the second request so the API doesn't know you want JSON for the second
-request and will responsd with XML since that's the default format. That causes
-problems so the API supports a custom `X-Expect-Override` header which instructs
-the browser to respond with a 200 status instead of a 303 so XMLHttpRequest
-doesn't detect the redirect. That allows the SDK to detect the redirect and respond
-accordingly. This also allows the SDK to track both the original URL and the
-effective (final) URL which normally isn't possible with XMLHttpRequest. Responses
+request and will respond with XML since that's the default format. Obtaining XML
+when you really want JSON is obviously problematic so the API supports a custom 
+`X-Expect-Override` header which instructs the browser to respond with a 200 
+status instead of a 303 so XMLHttpRequest doesn't detect the redirect. 
+That allows the SDK to detect the redirect and respond accordingly. 
+This also allows the SDK to track both the original URL and the effective 
+(final) URL which normally isn't possible with XMLHttpRequest. Responses
 from redirected requests will have the `redirected` property set to `true`.
 
+Second, we don't always want to follow redirects. For example, if a person
+portrait exists the API will respond with a 307 redirect to the image file.
+However all we usually want is the URL so that we can add insert the URL into
+HTML and have the browser download the image.
+
+Due to the two issue described above, the SDK defaults to no special handling
+of redirects. In node all redirect responses will be returned to the response
+callbacks and in the browser all redirects will be transparently followed by
+the browser. The default behavior can be modified by setting request options.
+
+* `expectRedirect`: When `true` the `X-Expect-Override` header will be set on the
+request which causes the API to respond with a 200 for redirects.
+* `followRedirect`: When `true` the SDK will automatically follow a redirect.
+
+__It gets worse.__ Redirects are not allowed with CORS requests which require a 
+preflight OPTIONS request thus you wil always want to set the `expectRedirect` 
+header to true in the browser. But you can't do that because the API honors the 
+`X-Expect-Override` header for 304s as well. That is problematic when requesting
+persons because your browser will cache the first response then send a 
+`If-none-match` request the second time which the API would reply to with a 304
+and an empty body but instead sends a 200 with the empty body but the browser
+doesn't understand that it's a cached response thus the response is resolved
+without a body. That's not what you want.
+
 ### Throttling
 
 The SDK will automatically retry throttled requests and obey the throttling

src/Request.js

@@ -6,12 +6,39 @@
  * @param {Function} callback
  */
 var Request = function(url, options, callback){
+
+  // Inititialize and set defaults
   this.url = url;
-  this.method = options.method || 'GET';
-  this.headers = options.headers ? JSON.parse(JSON.stringify(options.headers)) : {};
-  this.body = options.body;
-  this.retries = options.retries || 0;
   this.callback = callback || function(){};
+  this.method = 'GET';
+  this.headers = {};
+  this.retries = 0;
+  this.options = {};
+
+  // Process request options. We use a for loop so that we can stuff all
+  // non-standard options into the options object on the reuqest.
+  var opt;
+  for(opt in options){
+    if(options.hasOwnProperty(opt)){
+      switch(opt){
+
+        case 'method':
+        case 'body':
+        case 'retries':
+          this[opt] = options[opt];
+          break;
+
+        case 'headers':
+          // We copy the headers object so that we don't have to worry about the developer
+          // and the SDK stepping on each other's toes by modifying the headers object.
+          this.headers = JSON.parse(JSON.stringify(options.headers));
+          break;
+
+        default:
+          this.options[opt] = options[opt];
+      }
+    }
+  }
 };
 
 /**

src/middleware/request/disableAutomaticRedirects.js

@@ -1,6 +1,13 @@
-// Disable automatic redirects
+/**
+ * Disable automatic redirects. Useful client-side so that the browser doesn't
+ * automatically follow 3xx redirects; that causes problems if the browser
+ * doesn't replay all request options such as the Accept header.
+ * 
+ * This middleware is enabled per request by setting the `expectRedirect` request
+ * option to `true`.
+ */
 module.exports = function(client, request, next){
-  if(!request.hasHeader('X-Expect-Override') && request.isPlatform()){
+  if(request.options.expectRedirect && !request.hasHeader('X-Expect-Override') && request.isPlatform()){
     request.setHeader('X-Expect-Override', '200-ok');
   }
   next();

src/middleware/response/redirect.js

@@ -1,6 +1,13 @@
+/**
+ * Automatically follow a redirect. This behavior is optional because you don't
+ * allways want to follow redirects such as when requesting a person's profile.
+ * 
+ * This middleware is enabled per request by setting the `followRedirect` request 
+ * option to true.
+ */
 module.exports = function(client, request, response, next){
   var location = response.headers['location'];
-  if(response.statusCode === 200 && location && location !== request.url ){
+  if(request.options.followRedirect && location && location !== request.url ){
     var originalUrl = request.url;
     request.url = response.headers['location'];
     client._execute(request, function(error, response){

src/xhrHandler.js

@@ -25,12 +25,14 @@ module.exports = function(request, callback){
   xhr.onload = function(){
     var response = createResponse(xhr, request);
     setTimeout(function(){
+      console.log('onload');
       callback(null, response);
     });
   };
 
   // Attach error handler
   xhr.onerror = function(error){
+    console.log('error');
     setTimeout(function(){
       callback(error);
     });

test/browser.js

@@ -67,4 +67,43 @@ describe('browser', function(){
     });
   });
 
+  it('redirect', function(done){
+    this.timeout(10000);
+    nockBack('browserOauthPassword.json', function(nockDone){
+      client.oauthPassword(sandbox.username, sandbox.password, function(error, response){
+        nockDone();
+        check(function(error){
+          if(error){
+            done(error);
+          }
+        }, function(){
+          assert.isDefined(response);
+          assert.equal(response.statusCode, 200);
+          assert.isDefined(response.data);
+          assert.isDefined(response.data.token);
+          nockBack('browserRedirect.json', function(nockDone){
+            createPerson(client, function(personId){
+              client.get('/platform/tree/current-person', {
+                expectRedirect: true,
+                followRedirect: true
+              }, function(error, response){
+                nockDone();
+                check(done, function(){
+                  assert.isDefined(response);
+                  assert.equal(response.statusCode, 200);
+                  assert.isDefined(response.data);
+                  assert.isArray(response.data.persons);
+                  assert(response.redirected);
+                  assert.isDefined(response.originalUrl);
+                  assert.isDefined(response.effectiveUrl);
+                  assert(response.originalUrl !== response.effectiveUrl);
+                });
+              });
+            });
+          });
+        });
+      });
+    });
+  });
+
 });

test/node.js

@@ -93,7 +93,9 @@ describe('node', function(){
 
     it('redirect', function(done){
       nockBack('redirect.json', function(nockDone){
-        client.get('/platform/tree/current-person', function(error, response){
+        client.get('/platform/tree/current-person', {
+          followRedirect: true
+        }, function(error, response){
           nockDone();
           check(done, function(){
             assert.isDefined(response);

test/responses/browserOauthPassword.json

@@ -6,20 +6,20 @@
         "body": "grant_type=password&client_id=a02j000000JBxOxAAL&username=sdktester&password=1234sdkpass",
         "status": 200,
         "response": {
-            "token": "USYSFE9C8BF6A44C254CE62B706FB33215CD_idses-int01.a.fsglobal.net",
+            "token": "USYS996204AFB923618109AD7D3B316F694D_idses-int02.a.fsglobal.net",
             "token_type": "family_search",
-            "access_token": "USYSFE9C8BF6A44C254CE62B706FB33215CD_idses-int01.a.fsglobal.net"
+            "access_token": "USYS996204AFB923618109AD7D3B316F694D_idses-int02.a.fsglobal.net"
         },
         "headers": {
             "server": "Apache-Coyote/1.1",
             "expires": "Tue, 03 Jul 2001 06:00:00 GMT",
-            "last-modified": "Fri Nov 18 17:38:06 GMT+00:00 2016",
+            "last-modified": "Tue Nov 29 20:45:25 GMT+00:00 2016",
             "cache-control": "no-store, no-cache, must-revalidate, max-age=0, post-check=0, pre-check=0",
             "pragma": "no-cache",
             "content-type": "application/json;charset=ISO-8859-1",
             "content-language": "en",
             "content-length": "185",
-            "date": "Fri, 18 Nov 2016 17:38:06 GMT",
+            "date": "Tue, 29 Nov 2016 20:45:24 GMT",
             "connection": "close",
             "access-control-allow-methods": "OPTIONS, HEAD, GET, PUT, POST, DELETE",
             "access-control-allow-headers": "Accept, Accept-Charset, Accept-Encoding, Accept-Language, Accept-Datetime, Authorization, Cache-Control, Connection, Content-Length, Content-Md5, Content-Type, Date, Expect, From, Host, If-Match, If-Modified-Since, If-None-Match, If-Range, If-Unmodified-Since, Location, Origin, Pragma, Range, Referer, SingularityJSHeader, Te, User-Agent, Warning, X-Expect-Override, X-Reason, X-Requested-With, X-FS-Feature-Tag",