From 3eef695bc0f0b4548c4fa10ccd30b54f945df5ab Mon Sep 17 00:00:00 2001
From: Jacob Heun <jacobheun@gmail.com>
Date: Wed, 21 Aug 2019 16:44:30 +0200
Subject: [PATCH] fix: improve config defaults (#409)

This removes defaults from superstruct and instead uses
mergeOptions to deeply set the defaults on configuration.
This ensures that defaults are properly set.

This is a step toward removing superstruct altogether, #406,
but it is still being used for basic type validation.
---
 package.json        |  2 +-
 src/config.js       | 87 ++++++++++++++++++++++-----------------------
 test/config.spec.js | 17 +++++++++
 test/pubsub.node.js |  8 ++---
 4 files changed, 64 insertions(+), 50 deletions(-)

diff --git a/package.json b/package.json
index c3e55b0d41..0707248314 100644
--- a/package.json
+++ b/package.json
@@ -56,6 +56,7 @@
     "libp2p-crypto": "~0.16.1",
     "libp2p-websockets": "^0.12.2",
     "mafmt": "^6.0.7",
+    "merge-options": "^1.0.1",
     "moving-average": "^1.0.0",
     "multiaddr": "^6.1.0",
     "multistream-select": "~0.14.6",
@@ -99,7 +100,6 @@
     "libp2p-websocket-star": "~0.10.2",
     "libp2p-websocket-star-rendezvous": "~0.4.1",
     "lodash.times": "^4.3.2",
-    "merge-options": "^1.0.1",
     "nock": "^10.0.6",
     "portfinder": "^1.0.20",
     "pull-goodbye": "0.0.2",
diff --git a/src/config.js b/src/config.js
index e4b68559dc..587167cf5f 100644
--- a/src/config.js
+++ b/src/config.js
@@ -1,8 +1,43 @@
 'use strict'
 
+const mergeOptions = require('merge-options')
 const { struct, superstruct } = require('superstruct')
 const { optional, list } = struct
 
+const DefaultConfig = {
+  connectionManager: {
+    minPeers: 25
+  },
+  config: {
+    dht: {
+      enabled: false,
+      kBucketSize: 20,
+      randomWalk: {
+        enabled: false, // disabled waiting for https://github.com/libp2p/js-libp2p-kad-dht/issues/86
+        queriesPerPeriod: 1,
+        interval: 300e3,
+        timeout: 10e3
+      }
+    },
+    peerDiscovery: {
+      autoDial: true
+    },
+    pubsub: {
+      enabled: true,
+      emitSelf: true,
+      signMessages: true,
+      strictSigning: true
+    },
+    relay: {
+      enabled: true,
+      hop: {
+        enabled: false,
+        active: false
+      }
+    }
+  }
+}
+
 // Define custom types
 const s = superstruct({
   types: {
@@ -35,50 +70,15 @@ const modulesSchema = s({
 })
 
 const configSchema = s({
-  peerDiscovery: s('object', {
-    autoDial: true
-  }),
-  relay: s({
-    enabled: 'boolean',
-    hop: optional(s({
-      enabled: 'boolean',
-      active: 'boolean'
-    }, {
-      // HOP defaults
-      enabled: false,
-      active: false
-    }))
-  }, {
-    // Relay defaults
-    enabled: true
-  }),
-  // DHT config
-  dht: s('object?', {
-    // DHT defaults
-    enabled: false,
-    kBucketSize: 20,
-    randomWalk: {
-      enabled: false, // disabled waiting for https://github.com/libp2p/js-libp2p-kad-dht/issues/86
-      queriesPerPeriod: 1,
-      interval: 300e3,
-      timeout: 10e3
-    }
-  }),
-  // Pubsub config
-  pubsub: s('object?', {
-    // Pubsub defaults
-    enabled: true,
-    emitSelf: true,
-    signMessages: true,
-    strictSigning: true
-  })
-}, {})
+  peerDiscovery: 'object?',
+  relay: 'object?',
+  dht: 'object?',
+  pubsub: 'object?'
+})
 
 const optionsSchema = s({
   switch: 'object?',
-  connectionManager: s('object', {
-    minPeers: 25
-  }),
+  connectionManager: 'object?',
   datastore: 'object?',
   peerInfo: 'object',
   peerBook: 'object?',
@@ -87,6 +87,7 @@ const optionsSchema = s({
 })
 
 module.exports.validate = (opts) => {
+  opts = mergeOptions(DefaultConfig, opts)
   const [error, options] = optionsSchema.validate(opts)
 
   // Improve errors throwed, reduce stack by throwing here and add reason to the message
@@ -99,9 +100,5 @@ module.exports.validate = (opts) => {
     }
   }
 
-  if (options.config.peerDiscovery.autoDial === undefined) {
-    options.config.peerDiscovery.autoDial = true
-  }
-
   return options
 }
diff --git a/test/config.spec.js b/test/config.spec.js
index 7c1ce53058..146f2e3dae 100644
--- a/test/config.spec.js
+++ b/test/config.spec.js
@@ -125,6 +125,15 @@ describe('configuration', () => {
             interval: 1000,
             enabled: true
           }
+        },
+        dht: {
+          enabled: false
+        },
+        relay: {
+          enabled: true
+        },
+        pubsub: {
+          enabled: true
         }
       }
     }
@@ -292,6 +301,14 @@ describe('configuration', () => {
           }
         },
         dht: {
+          kBucketSize: 20,
+          enabled: false,
+          randomWalk: {
+            enabled: false,
+            queriesPerPeriod: 1,
+            interval: 300000,
+            timeout: 10000
+          },
           selectors,
           validators
         }
diff --git a/test/pubsub.node.js b/test/pubsub.node.js
index 613d631fd5..43cab4c7b0 100644
--- a/test/pubsub.node.js
+++ b/test/pubsub.node.js
@@ -376,7 +376,7 @@ describe('.pubsub', () => {
         constructor (node, config) {
           expect(config).to.be.eql({
             enabled: true,
-            selfEmit: false,
+            emitSelf: false,
             signMessages: false,
             strictSigning: false
           }).mark()
@@ -390,7 +390,7 @@ describe('.pubsub', () => {
         config: {
           pubsub: {
             enabled: true,
-            selfEmit: false,
+            emitSelf: false,
             signMessages: false,
             strictSigning: false
           }
@@ -408,7 +408,7 @@ describe('.pubsub', () => {
         constructor (node, config) {
           expect(config).to.be.eql({
             enabled: true,
-            selfEmit: true,
+            emitSelf: true,
             signMessages: true,
             strictSigning: true
           }).mark()
@@ -422,7 +422,7 @@ describe('.pubsub', () => {
         config: {
           pubsub: {
             enabled: true,
-            selfEmit: true,
+            emitSelf: true,
             signMessages: true,
             strictSigning: true
           }