Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Remove broken offline special-casing when retrieving cached data #1034

Merged
merged 7 commits into from Jan 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 19 additions & 32 deletions lib/atom-io-client.coffee
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 1 addition & 2 deletions spec/atom-io-client-spec.coffee
Expand Up @@ -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', ->

Expand Down
11 changes: 5 additions & 6 deletions spec/package-detail-view-spec.coffee
Expand Up @@ -64,17 +64,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)
Expand Down