UX fixes around embedded js-ipfs and automatic mode

- suspend automatic mode while embedded node is active - redirect to public gateway instead of custom one when running js-ipfs - fix UX when starting with external API being offline (#373) - extract some reusable feature-detection functions - open public gateway instead of custom one when uploading a file using embedded js-ipfs under Firefox - some tests
ipfs · Feb 11, 2018 · e6eb2ba · e6eb2ba
1 parent 888d3cf
commit e6eb2ba
Show file tree

Hide file tree

Showing 9 changed files with 343 additions and 35 deletions.
diff --git a/add-on/manifest.json b/add-on/manifest.json
@@ -15,7 +15,7 @@
     "applications": {
         "gecko": {
             "id": "[email protected]",
-            "strict_min_version": "57.0"
+            "strict_min_version": "58.0"
         }
     },
 

diff --git a/add-on/src/lib/dns-link.js b/add-on/src/lib/dns-link.js
@@ -3,6 +3,7 @@
 
 const IsIpfs = require('is-ipfs')
 const { LRUMap } = require('lru_map')
+const { embeddedNodeIsActive } = require('./state')
 
 module.exports = function createDnsLink (getState) {
   const cache = new LRUMap(1000)
@@ -84,10 +85,15 @@ module.exports = function createDnsLink (getState) {
       }
     },
 
-    redirectToIpnsPath (url) {
-      const fqdn = url.hostname
-      url.protocol = getState().gwURL.protocol
-      url.host = getState().gwURL.host
+    redirectToIpnsPath (originalUrl) {
+      // TODO: redirect to `ipns://` if browserWithNativeProtocol() === true
+      const fqdn = originalUrl.hostname
+      const state = getState()
+      const gwUrl = embeddedNodeIsActive(state) ? state.pubGwURL : state.gwURL
+      const url = new URL(originalUrl)
+      url.protocol = gwUrl.protocol
+      url.host = gwUrl.host
+      url.port = gwUrl.port
       url.pathname = `/ipns/${fqdn}${url.pathname}`
       return { redirectUrl: url.toString() }
     }

diff --git a/add-on/src/lib/ipfs-companion.js b/add-on/src/lib/ipfs-companion.js
@@ -3,7 +3,7 @@
 
 const browser = require('webextension-polyfill')
 const { optionDefaults, storeMissingOptions } = require('./options')
-const { initState } = require('./state')
+const { initState, offlinePeerCount, inFirefox, browserWithNativeProtocol, embeddedNodeIsActive } = require('./state')
 const { createIpfsPathValidator, urlAtPublicGw } = require('./ipfs-path')
 const createDnsLink = require('./dns-link')
 const { createRequestModifier } = require('./ipfs-request')
@@ -28,7 +28,6 @@ module.exports = async function init () {
   var contextMenus
   var apiStatusUpdateInterval
   var ipfsProxy
-  const offlinePeerCount = -1
   const idleInSecs = 5 * 60
   const browserActionPortName = 'browser-action-port'
 
@@ -74,11 +73,11 @@ module.exports = async function init () {
     browser.runtime.onMessage.addListener(onRuntimeMessage)
     browser.runtime.onConnect.addListener(onRuntimeConnect)
    // browser.protocol exists only in Brave
-    if (browser.protocol && browser.protocol.registerStringProtocol) {
+    if (browserWithNativeProtocol()) {
       console.log(`[ipfs-companion] registerStringProtocol available. Adding ipfs:// handler`)
       browser.protocol.registerStringProtocol('ipfs', createIpfsUrlProtocolHandler(() => ipfs))
     } else {
-      console.log(`[ipfs-companion] registerStringProtocol not available, native protocol will not be registered`, browser.protocol)
+      console.log('[ipfs-companion] browser.protocol.registerStringProtocol not available, native protocol will not be registered')
     }
   }
 
@@ -181,10 +180,6 @@ module.exports = async function init () {
   // GUI
   // ===================================================================
 
-  function inFirefox () {
-    return !!navigator.userAgent.match('Firefox')
-  }
-
   function preloadAtPublicGateway (path) {
     // asynchronous HTTP HEAD request preloads triggers content without downloading it
     return new Promise((resolve, reject) => {
@@ -251,23 +246,23 @@ module.exports = async function init () {
   }
 
   // TODO: feature detect and push to client type specific modules.
-  function getIpfsPathAndLocalAddress (hash) {
+  function getIpfsPathAndNativeAddress (hash) {
     const path = `/ipfs/${hash}`
-    if (state.ipfsNodeType === 'embedded' && browser && browser.protocol && browser.protocol.registerStringProtocol) {
-      return {path, localAddress: `ipfs://${hash}`}
+    if (embeddedNodeIsActive(state) && browserWithNativeProtocol()) {
+      return {path, nativeAddress: `ipfs://${hash}`}
     } else {
-      // Use the chosen gateway... local or public
-      const url = new URL(path, state.gwURLString).toString()
-      return {path, localAddress: url}
+      // open at public GW (will be redirected to local elsewhere, if enabled)
+      const url = new URL(path, state.pubGwURLString).toString()
+      return {path, nativeAddress: url}
     }
   }
 
   function uploadResultHandler (result) {
     result.forEach(function (file) {
       if (file && file.hash) {
-        const {path, localAddress} = getIpfsPathAndLocalAddress(file.hash)
+        const {path, nativeAddress} = getIpfsPathAndNativeAddress(file.hash)
         browser.tabs.create({
-          'url': localAddress
+          'url': nativeAddress
         })
         console.info('[ipfs-companion] successfully stored', path)
         if (state.preloadAtPublicGateway) {
@@ -350,11 +345,14 @@ module.exports = async function init () {
   }
 
   async function apiStatusUpdate () {
+    // update peer count
     let oldPeerCount = state.peerCount
     state.peerCount = await getSwarmPeerCount()
-    state.repoStats = await getRepoStats()
     updatePeerCountDependentStates(oldPeerCount, state.peerCount)
-    sendStatusUpdateToBrowserAction()
+    // update repo stats
+    state.repoStats = await getRepoStats()
+    // trigger pending updates
+    await sendStatusUpdateToBrowserAction()
   }
 
   function updatePeerCountDependentStates (oldPeerCount, newPeerCount) {
@@ -482,11 +480,11 @@ module.exports = async function init () {
     // enable/disable gw redirect based on API going online or offline
     // newPeerCount === -1 currently implies node is offline.
     // TODO: use `node.isOnline()` if available (js-ipfs)
-    if (state.automaticMode) {
-      if (oldPeerCount === -1 && newPeerCount > -1 && !state.redirect) {
+    if (state.automaticMode && !embeddedNodeIsActive(state)) {
+      if (oldPeerCount === offlinePeerCount && newPeerCount > offlinePeerCount && !state.redirect) {
         browser.storage.local.set({useCustomGateway: true})
           .then(() => notify('notify_apiOnlineTitle', 'notify_apiOnlineAutomaticModeMsg'))
-      } else if (oldPeerCount > -1 && newPeerCount === -1 && state.redirect) {
+      } else if (newPeerCount === offlinePeerCount && state.redirect) {
         browser.storage.local.set({useCustomGateway: false})
           .then(() => notify('notify_apiOfflineTitle', 'notify_apiOfflineAutomaticModeMsg'))
       }

diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js
@@ -3,6 +3,7 @@
 
 const IsIpfs = require('is-ipfs')
 const { urlAtPublicGw } = require('./ipfs-path')
+const { embeddedNodeIsActive } = require('./state')
 
 function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
   return function modifyRequest (request) {
@@ -35,6 +36,12 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
       }
     }
 
+    // skip requests to the public gateway if embedded node is running (otherwise we have too much recursion)
+    if (embeddedNodeIsActive(state) && request.url.startsWith(state.pubGwURLString)) {
+      return
+      // TODO: do not skip and redirect to `ipfs://` and `ipns://` if browserWithNativeProtocol() === true
+    }
+
     // handle redirects to custom gateway
     if (state.redirect) {
       // Ignore preload requests
@@ -43,7 +50,7 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
       }
       // Detect valid /ipfs/ and /ipns/ on any site
       if (ipfsPathValidator.publicIpfsOrIpnsResource(request.url)) {
-        return redirectToCustomGateway(request.url, state.gwURL)
+        return redirectToIpfsTransport(request.url, state)
       }
       // Look for dnslink in TXT records of visited sites
       if (state.dnslink && dnsLink.isDnslookupSafeForURL(request.url)) {
@@ -55,7 +62,9 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
 
 exports.createRequestModifier = createRequestModifier
 
-function redirectToCustomGateway (requestUrl, gwUrl) {
+function redirectToIpfsTransport (requestUrl, state) {
+  // TODO: redirect to `ipfs://` if browserWithNativeProtocol() === true
+  const gwUrl = embeddedNodeIsActive(state) ? state.pubGwURL : state.gwURL
   const url = new URL(requestUrl)
   url.protocol = gwUrl.protocol
   url.host = gwUrl.host

diff --git a/add-on/src/lib/state.js b/add-on/src/lib/state.js
@@ -1,10 +1,15 @@
 'use strict'
-/* eslint-env browser */
+/* eslint-env browser, webextensions */
+
+// The State
+// ===================================================================
+const offlinePeerCount = -1
 
 function initState (options) {
   const state = {}
   // we store the most used values in optimized form
   // to minimize performance impact on overall browsing experience
+  state.peerCount = offlinePeerCount
   state.ipfsNodeType = options.ipfsNodeType
   state.pubGwURL = new URL(options.publicGatewayUrl)
   state.pubGwURLString = state.pubGwURL.toString()
@@ -22,4 +27,21 @@ function initState (options) {
   return state
 }
 
+// Browser Feature Detection
+// ===================================================================
+
+exports.inFirefox = function () {
+  // TODO: we really need to change this to something more robust
+  return !!navigator.userAgent.match('Firefox')
+}
+
+exports.browserWithNativeProtocol = function () {
+  return browser && browser.protocol && browser.protocol.registerStringProtocol
+}
+
+exports.embeddedNodeIsActive = function (state) {
+  return state.ipfsNodeType === 'embedded'
+}
+
 exports.initState = initState
+exports.offlinePeerCount = offlinePeerCount
diff --git a/test/functional/lib/dns-link.test.js b/test/functional/lib/dns-link.test.js
@@ -1,18 +1,44 @@
 'use strict'
-const { describe, it } = require('mocha')
+const { describe, it, before, after } = require('mocha')
 const { expect } = require('chai')
 const { URL } = require('url')
 const createDnsLink = require('../../../add-on/src/lib/dns-link')
 
 // https://github.com/ipfs/ipfs-companion/issues/303
 describe('DNSLINK', function () {
-  describe('redirectToIpnsPath(url)', function () {
+  before(() => {
+    global.URL = URL
+  })
+
+  describe('redirectToIpnsPath(url) with external gateway', function () {
     it('should return IPNS path at a custom gateway', function () {
       const url = new URL('http://ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
-      const getState = () => ({ gwURL: new URL('http://127.0.0.1:8080') })
+      const getState = () => ({
+        gwURL: new URL('http://127.0.0.1:8080'),
+        pubGwURL: new URL('https://ipfs.io'),
+        ipfsNodeType: 'external'
+      })
       const dnsLink = createDnsLink(getState)
       expect(dnsLink.redirectToIpnsPath(url).redirectUrl)
         .to.equal('http://127.0.0.1:8080/ipns/ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
     })
   })
+
+  describe('redirectToIpnsPath(url) with embedded gateway', function () {
+    it('should return IPNS path at a public gateway', function () {
+      const url = new URL('http://ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+      const getState = () => ({
+        gwURL: new URL('http://127.0.0.1:8080'),
+        pubGwURL: new URL('https://ipfs.io'),
+        ipfsNodeType: 'embedded'
+      })
+      const dnsLink = createDnsLink(getState)
+      expect(dnsLink.redirectToIpnsPath(url).redirectUrl)
+        .to.equal('https://ipfs.io/ipns/ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+    })
+  })
+
+  after(() => {
+    delete global.URL
+  })
 })