Browse Source

Fixing code review issues: adding a partition options and making the session option only takes Session objects.

ali.ibrahim 8 years ago
parent
commit
b44d5290e2
4 changed files with 40 additions and 17 deletions
  1. 11 6
      atom/browser/api/atom_api_url_request.cc
  2. 6 2
      docs/api/net.md
  3. 19 5
      lib/browser/api/net.js
  4. 4 4
      spec/api-net-spec.js

+ 11 - 6
atom/browser/api/atom_api_url_request.cc

@@ -175,16 +175,21 @@ URLRequest::~URLRequest() {
 mate::WrappableBase* URLRequest::New(mate::Arguments* args) {
   v8::Local<v8::Object> options;
   args->GetNext(&options);
-  mate::Dictionary dict(args->isolate(), options);
+  auto isolate = args->isolate();
+  mate::Dictionary dict(isolate, options);
   std::string method;
   dict.Get("method", &method);
   std::string url;
   dict.Get("url", &url);
-  std::string session_name;
-  dict.Get("session", &session_name);
-
-  auto session = Session::FromPartition(args->isolate(), session_name);
-
+  std::string partition;
+  mate::Handle<api::Session> session;
+  if (dict.Get("session", &session)) {
+  } else if (dict.Get("partition", &partition)) {
+    session = Session::FromPartition(isolate, partition);
+  } else {
+    // Use the default session if not specified.
+    session = Session::FromPartition(isolate, "");
+  }
   auto browser_context = session->browser_context();
   auto api_url_request = new URLRequest(args->isolate(), args->GetThis());
   auto atom_url_request =

+ 6 - 2
docs/api/net.md

@@ -82,8 +82,12 @@ following properties:
 method.
   * `url` String (optional) - The request URL. Must be provided in the absolute
 form with the protocol scheme specified as http or https.
-  * `session` String (optional) - The name of the [`Session`](session.md) 
-instance with which the request is associated. Defaults to the empty string.
+  * `session` Object (optional) - The [`Session`](session.md) instance with
+which the request is associated.
+  * `partition` String (optional) - The name of the [`partition`](session.md)
+  with which the request is associated. Defaults to the empty string. The
+`session` option prevails on `partition`. Thus if a `session` is explicitly
+specified, `partition` is ignored.
   * `protocol` String (optional) - The protocol scheme in the form 'scheme:'.
 Currently supported values are 'http:' or 'https:'. Defaults to 'http:'.
   * `host` String (optional) - The server host provided as a concatenation of

+ 19 - 5
lib/browser/api/net.js

@@ -5,6 +5,7 @@ const {EventEmitter} = require('events')
 const util = require('util')
 const {Readable} = require('stream')
 const {app} = require('electron')
+const {session} = require('electron')
 const {net, Net} = process.atomBinding('net')
 const {URLRequest} = net
 
@@ -155,12 +156,25 @@ class ClientRequest extends EventEmitter {
       urlStr = url.format(urlObj)
     }
 
-    const sessionName = options.session || ''
-    let urlRequest = new URLRequest({
+    let urlRequestOptions = {
       method: method,
-      url: urlStr,
-      session: sessionName
-    })
+      url: urlStr
+    }
+    if (options.session) {
+      if (options.session instanceof session.Session) {
+        urlRequestOptions.session = options.session
+      } else {
+        throw new TypeError('`session` should be an instance of the Session class.')
+      }
+    } else if (options.partition) {
+      if (typeof options.partition === 'string') {
+        urlRequestOptions.partition = options.partition
+      } else {
+        throw new TypeError('`partition` should be an a string.')
+      }
+    }
+
+    let urlRequest = new URLRequest(urlRequestOptions)
 
     // Set back and forward links.
     this.urlRequest = urlRequest

+ 4 - 4
spec/api-net-spec.js

@@ -797,10 +797,10 @@ describe('net module', function () {
       urlRequest.end()
     })
 
-    it('should to able to create and intercept a request on a custom session', function (done) {
+    it('should to able to create and intercept a request using a custom parition name', function (done) {
       const requestUrl = '/requestUrl'
       const redirectUrl = '/redirectUrl'
-      const customSessionName = 'custom-session'
+      const customPartitionName = 'custom-partition'
       let requestIsRedirected = false
       server.on('request', function (request, response) {
         switch (request.url) {
@@ -821,7 +821,7 @@ describe('net module', function () {
           assert(false, 'Request should not be intercepted by the default session')
         })
 
-      let customSession = session.fromPartition(customSessionName, {
+      let customSession = session.fromPartition(customPartitionName, {
         cache: false
       })
       let requestIsIntercepted = false
@@ -841,7 +841,7 @@ describe('net module', function () {
 
       const urlRequest = net.request({
         url: `${server.url}${requestUrl}`,
-        session: customSessionName
+        partition: customPartitionName
       })
       urlRequest.on('response', function (response) {
         assert.equal(response.statusCode, 200)