Cleaner URL handing (WIP)

Author: stianeikeland

src/client.coffee

@@ -34,11 +34,11 @@ class CancellationToken
 # HTTP Client for connecting to etcd servers
 class Client
 
-  constructor: (@hosts, @sslopts) ->
+  constructor: (@hosts, @options, @sslopts) ->
     @syncmsg = {}
 
   execute: (method, options, callback) =>
-    opt = _.defaults (_.clone options), defaultRequestOptions, { method: method }
+    opt = _.defaults (_.clone options), @options, defaultRequestOptions, { method: method }
     opt.clientOptions = _.defaults opt.clientOptions, defaultClientOptions
 
     servers = _.shuffle @hosts
@@ -59,7 +59,7 @@ class Client
   # Multiserver (cluster) executer
   _multiserverHelper: (servers, options, token, callback) =>
     host = _.first(servers)
-    options.url = "#{options.serverprotocol}://#{host}#{options.path}"
+    options.url = "#{host}#{options.path}"
 
     return if token.isAborted() # Aborted by user?
 

src/index.coffee

@@ -2,26 +2,30 @@ _          = require 'underscore'
 Watcher    = require './watcher'
 Client     = require './client'
 HttpsAgent = (require 'https').Agent
+URL        = require 'url-parse'
 
 # Etcd client for etcd protocol version 2
 class Etcd
 
   # Constructor, set etcd host and port.
   # For https: provide {ca, crt, key} as sslopts.
-  constructor: (host = '127.0.0.1', port = null, sslopts = null, client = null) ->
+  # constructor: (host = '127.0.0.1', port = null, sslopts = null, client = null) ->
 
-    if _.isArray(host)
-      @hosts = host
-      @sslopts = port
-      @client = sslopts
-    else
-      port ?= 4001
-      @hosts = ["#{host}:#{port}"]
-      @sslopts = sslopts
-      @client = client
+  #   if _.isArray(host)
+  #     @hosts = host
+  #     @sslopts = port
+  #     @client = sslopts
+  #   else
+  #     port ?= 4001
+  #     @hosts = ["#{host}:#{port}"]
+  #     @sslopts = sslopts
+  #     @client = client
 
-    @client ?= new Client(@hosts, @sslopts)
+  #   @client ?= new Client(@hosts, @sslopts)
 
+  constructor: (hosts = "127.0.0.1:4001", options = {}) ->
+    @hosts = @_cleanHostList hosts
+    @client = new Client(@hosts, options, null)
 
   # Set key to value
   # Usage:
@@ -230,15 +234,15 @@ class Etcd
 
   # Prepare request options
   _prepareOpts: (path, apiVersion = "/v2", value = null, allOpts = {}) ->
-    serverprotocol = if @sslopts? then "https" else "http"
+    # serverprotocol = if @sslopts? then "https" else "http"
 
     queryString = _.omit allOpts, 'maxRetries', 'synchronous'
 
     clientOptions = _.pick allOpts, 'maxRetries'
 
     opt = {
       path: "#{apiVersion}/#{path}"
-      serverprotocol: serverprotocol
+      # serverprotocol: serverprotocol
       json: true
       qs: queryString
       clientOptions: clientOptions
@@ -255,5 +259,14 @@ class Etcd
     else
       [options, callback]
 
+  # Make sure hosts is a list, make sure all have protocol added
+  # defaults to http
+  _cleanHostList: (hosts) ->
+    hostlist = if _.isArray(hosts) then hosts else [hosts]
+    hostlist.map (host) ->
+      url = new URL(host)
+      url.set 'protocol', 'http:' if url.protocol is ''
+      url.href
+
 
 exports = module.exports = Etcd

test/index.coffee

@@ -10,6 +10,10 @@ process.env.RUNNING_UNIT_TESTS = true
 getNock = (host = 'http://127.0.0.1:4001') ->
   nock host
 
+
+beforeEach () ->
+  nock.cleanAll()
+
 # Tests for utility functions
 
 describe 'Utility', ->
@@ -28,6 +32,57 @@ describe 'Utility', ->
         path: '/v2/keypath/key'
       }
 
+
+describe 'Connecting', ->
+
+  mock = (host = 'http://127.0.0.1:4001/') ->
+    nock(host)
+      .get('/v2/keys/key')
+      .reply(200, '{"action":"GET","key":"/key","value":"value","index":1}')
+
+  it 'should support empty constructor (localhost:4001)', (done) ->
+    etcd = new Etcd
+    m = mock()
+    etcd.get 'key', (err, val) ->
+      m.isDone().should.be.true
+      done err, val
+
+  it 'should support string as connect host', (done) ->
+    etcd = new Etcd "testhost.com:4009"
+    m = mock("http://testhost.com:4009")
+    etcd.get 'key', (err, val) ->
+      m.isDone().should.be.true
+      done err, val
+
+  it 'should support string prefixed by http:// as host', (done) ->
+    etcd = new Etcd "http://testhost.com:4009"
+    m = mock("http://testhost.com:4009")
+    etcd.get 'key', (err, val) ->
+      m.isDone().should.be.true
+      done err, val
+
+  it 'should support string postfixed by / as host', (done) ->
+    etcd = new Etcd "http://testhost.com:4009/"
+    m = mock("http://testhost.com:4009")
+    etcd.get 'key', (err, val) ->
+      m.isDone().should.be.true
+      done err, val
+
+  it 'should support array of strings as host', (done) ->
+    etcd = new Etcd ["http://testhost.com:4009"]
+    m = mock("http://testhost.com:4009")
+    etcd.get 'key', (err, val) ->
+      m.isDone().should.be.true
+      done err, val
+
+  it 'should support https strings', (done) ->
+    etcd = new Etcd ["https://testhost.com:1000"]
+    m = mock("https://testhost.com:1000")
+    etcd.get 'key', (err, val) ->
+      m.isDone().should.be.true
+      done err, val
+
+
 describe 'Basic functions', ->
 
   etcd = new Etcd
@@ -308,13 +363,7 @@ describe 'Multiserver/Cluster support', ->
 
   it 'should accept list of servers in constructor', ->
     etcd = new Etcd ['localhost:4001', 'localhost:4002']
-    etcd.getHosts().should.eql ['localhost:4001', 'localhost:4002']
-
-
-  it 'should accept host and port in constructor', ->
-    etcd = new Etcd 'localhost', 4001
-    etcd.getHosts().should.eql ['localhost:4001']
-
+    etcd.getHosts().should.eql ['http://localhost:4001', 'http://localhost:4002']
 
   it 'should try next server in list on http error', (done) ->
     path = '/v2/keys/foo'