Merge pull request #1034 from atom/wl-fix-caching

Remove broken offline special-casing when retrieving cached data

lib/atom-io-client.coffee

@@ -27,27 +27,27 @@ class AtomIoClient
   # caching.
   package: (name, callback) ->
     packagePath = "packages/#{name}"
-    @fetchFromCache packagePath, {}, (err, data) =>
-      if data
-        callback(null, data)
-      else
-        @request(packagePath, callback)
+    data = @fetchFromCache(packagePath)
+    if data
+      callback(null, data)
+    else
+      @request(packagePath, callback)
 
   featuredPackages: (callback) ->
     # TODO clean up caching copypasta
-    @fetchFromCache 'packages/featured', {}, (err, data) =>
-      if data
-        callback(null, data)
-      else
-        @getFeatured(false, callback)
+    data = @fetchFromCache 'packages/featured'
+    if data
+      callback(null, data)
+    else
+      @getFeatured(false, callback)
 
   featuredThemes: (callback) ->
     # TODO clean up caching copypasta
-    @fetchFromCache 'themes/featured', {}, (err, data) =>
-      if data
-        callback(null, data)
-      else
-        @getFeatured(true, callback)
+    data = @fetchFromCache 'themes/featured'
+    if data
+      callback(null, data)
+    else
+      @getFeatured(true, callback)
 
   getFeatured: (loadThemes, callback) ->
     # apm already does this, might as well use it instead of request i guess? The
@@ -97,27 +97,14 @@ class AtomIoClient
 
   # This could use a better name, since it checks whether it's appropriate to return
   # the cached data and pretends it's null if it's stale and we're online
-  fetchFromCache: (packagePath, options, callback) ->
-    unless callback
-      callback = options
-      options = {}
-
-    unless options.force
-      # Set `force` to true if we can't reach the network.
-      options.force = not @online()
-
+  fetchFromCache: (packagePath) ->
     cached = localStorage.getItem(@cacheKeyForPath(packagePath))
     cached = if cached then JSON.parse(cached)
-    if cached? and (not @online() or options.force or (Date.now() - cached.createdOn < @expiry))
-      cached ?= data: {}
-      callback(null, cached.data)
-    else if not cached? and not @online()
-      # The user hasn't requested this resource before and there's no way for us
-      # to get it to them so just hand back an empty object so callers don't crash
-      callback(null, {})
+    if cached? and (not @online() or Date.now() - cached.createdOn < @expiry)
+      return cached.data
     else
       # falsy data means "try to hit the network"
-      callback(null, null)
+      return null
 
   createAvatarCache: ->
     fs.makeTree(@getCachePath())

spec/atom-io-client-spec.coffee

@@ -14,8 +14,7 @@ describe "AtomIoClient", ->
   describe "request", ->
     it "fetches api json from cache if the network is unavailable", ->
       spyOn(@client, 'online').andReturn(false)
-      spyOn(@client, 'fetchFromCache').andCallFake (path, opts, cb) ->
-        cb(null, {})
+      spyOn(@client, 'fetchFromCache').andReturn({})
       spyOn(@client, 'request')
       @client.package 'test-package', ->
 

spec/package-detail-view-spec.coffee

@@ -65,17 +65,16 @@ describe "PackageDetailView", ->
     expect(view.element.querySelectorAll('.package-card').length).toBe(0)
 
   it "shows an error when package metadata cannot be loaded from the cache and the network is unavailable", ->
+    localStorage.removeItem('settings-view:packages/some-package')
+
     spyOn(AtomIoClient.prototype, 'online').andReturn(false)
-    spyOn(AtomIoClient.prototype, 'request').andCallThrough()
-    spyOn(AtomIoClient.prototype, 'fetchFromCache').andCallFake (path, opts, cb) ->
-      # this is the special case which happens when the data is not in the cache
-      # and there's no connectivity
-      cb(null, {})
+    spyOn(AtomIoClient.prototype, 'request').andCallFake (path, callback) ->
+      callback(new Error('getaddrinfo ENOENT atom.io:443'))
+    spyOn(AtomIoClient.prototype, 'fetchFromCache').andCallThrough()
 
     view = new PackageDetailView({name: 'some-package'}, new SettingsView(), packageManager, SnippetsProvider)
 
     expect(AtomIoClient.prototype.fetchFromCache).toHaveBeenCalled()
-    expect(AtomIoClient.prototype.request).not.toHaveBeenCalled()
 
     expect(view.refs.errorMessage.classList.contains('hidden')).not.toBe(true)
     expect(view.refs.loadingMessage.classList.contains('hidden')).toBe(true)