diff --git a/TODO b/TODO index d472f59e..94485f44 100644 --- a/TODO +++ b/TODO @@ -44,8 +44,6 @@ MEDIUM - retry cdrdao a few times when it had to load the tray -- getting cache results should depend on same drive/offset - - do some character mangling so trail of dead is not in a hidden dir HARD diff --git a/whipper/command/cd.py b/whipper/command/cd.py index 10a646f6..a9516e06 100644 --- a/whipper/command/cd.py +++ b/whipper/command/cd.py @@ -99,7 +99,6 @@ def do(self): self.ittoc = self.program.getFastToc(self.runner, self.device) # already show us some info based on this - self.program.getRipResult(self.ittoc.getCDDBDiscId()) print("CDDB disc id: %s" % self.ittoc.getCDDBDiscId()) self.mbdiscid = self.ittoc.getMusicBrainzDiscId() print("MusicBrainz disc id %s" % self.mbdiscid) @@ -499,8 +498,6 @@ def _ripIfNotRipped(number): self.itable.setFile(number, 1, trackResult.filename, self.itable.getTrackLength(number), number) - self.program.saveRipResult() - # check for hidden track one audio htoa = self.program.getHTOA() if htoa: @@ -541,8 +538,6 @@ def _ripIfNotRipped(number): accurip.print_report(self.program.result) - self.program.saveRipResult() - self.program.writeLog(discName, self.logger) diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py index 4e984a99..8f504fdd 100644 --- a/whipper/common/accurip.py +++ b/whipper/common/accurip.py @@ -21,12 +21,9 @@ import struct import whipper -from os import makedirs -from os.path import dirname, exists, join from urllib.error import URLError, HTTPError from urllib.request import urlopen, Request -from whipper.common import directory from whipper.program.arc import accuraterip_checksum import logging @@ -34,7 +31,6 @@ ACCURATERIP_URL = "http://www.accuraterip.com/accuraterip/" -_CACHE_DIR = join(directory.cache_path(), 'accurip') class EntryNotFound(Exception): @@ -142,34 +138,13 @@ def _download_entry(path): logger.error('error retrieving AccurateRip entry: %s', e) -def _save_entry(raw_entry, path): - logger.debug('saving AccurateRip entry to %s', path) - try: - makedirs(dirname(path), exist_ok=True) - except OSError as e: - logger.error('could not save entry to %s: %s', path, e) - return - with open(path, 'wb') as f: - f.write(raw_entry) - - def get_db_entry(path): """ - Retrieve cached AccurateRip disc entry as array of _AccurateRipResponses. - - Downloads entry from accuraterip.com on cache fault. + Download entry from accuraterip.com. ``path`` is in the format of the output of ``table.accuraterip_path()``. """ - cached_path = join(_CACHE_DIR, path) - if exists(cached_path): - logger.debug('found accuraterip entry at %s', cached_path) - with open(cached_path, 'rb') as f: - raw_entry = f.read() - else: - raw_entry = _download_entry(path) - if raw_entry: - _save_entry(raw_entry, cached_path) + raw_entry = _download_entry(path) if not raw_entry: logger.warning('entry not found in AccurateRip database') raise EntryNotFound diff --git a/whipper/common/cache.py b/whipper/common/cache.py deleted file mode 100644 index 821c8c10..00000000 --- a/whipper/common/cache.py +++ /dev/null @@ -1,218 +0,0 @@ -# -*- Mode: Python; test-case-name: whipper.test.test_common_cache -*- -# vi:si:et:sw=4:sts=4:ts=4 - -# Copyright (C) 2009 Thomas Vander Stichele - -# This file is part of whipper. -# -# whipper is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# whipper is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with whipper. If not, see . - -import os -import os.path -import glob -import tempfile -import shutil - -from whipper.result import result -from whipper.common import directory - -import logging -logger = logging.getLogger(__name__) - - -class Persister: - """ - I wrap an optional pickle to persist an object to disk. - - Instantiate me with a path to automatically unpickle the object. - Call persist to store the object to disk; it will get stored if it - changed from the on-disk object. - - :ivar object: the persistent object - """ - - def __init__(self, path=None, default=None): - """ - If path is not given, the object will not be persisted. - - This allows code to transparently deal with both persisted and - non-persisted objects, since the persist method will just end up - doing nothing. - """ - self._path = path - self.object = None - - self._unpickle(default) - - def persist(self, obj=None): - """ - Persist the given obj if we have a persist. path and the obj changed. - - If object is not given, re-persist our object, always. - If object is given, only persist if it was changed. - """ - # don't pickle if it's already ok - if obj and obj == self.object: - return - - # store the object on ourselves if not None - if obj is not None: - self.object = obj - - # don't pickle if there is no path - if not self._path: - return - - # default to pickling our object again - if obj is None: - obj = self.object - - # pickle - self.object = obj - (fd, path) = tempfile.mkstemp(suffix='.whipper.pickle') - handle = os.fdopen(fd, 'wb') - import pickle - pickle.dump(obj, handle, 2) - handle.close() - # do an atomic move - shutil.move(path, self._path) - logger.debug('saved persisted object to %r', self._path) - - def _unpickle(self, default=None): - self.object = default - - if not self._path: - return - - if not os.path.exists(self._path): - return - - with open(self._path, 'rb') as handle: - import pickle - try: - self.object = pickle.load(handle) - logger.debug('loaded persisted object from %r', self._path) - # FIXME: catching too general exception (Exception) - except Exception as e: - # can fail for various reasons; in that case, pretend we didn't - # load it - logger.debug(e) - - def delete(self): - self.object = None - os.unlink(self._path) - - -class PersistedCache: - """Wrap a directory of persisted objects.""" - - path = None - - def __init__(self, path): - self.path = path - os.makedirs(self.path, exist_ok=True) - - def _getPath(self, key): - return os.path.join(self.path, '%s.pickle' % key) - - def get(self, key): - """Return the persister for the given key.""" - persister = Persister(self._getPath(key)) - if persister.object: - if hasattr(persister.object, 'instanceVersion'): - o = persister.object - if o.instanceVersion < o.__class__.classVersion: - logger.debug('key %r persisted object version %d ' - 'is outdated', key, o.instanceVersion) - persister.object = None - # FIXME: don't delete old objects atm - # persister.delete() - - return persister - - -class ResultCache: - - def __init__(self, path=None): - self._path = path or directory.cache_path('result') - self._pcache = PersistedCache(self._path) - - def getRipResult(self, cddbdiscid, create=True): - """ - Get the persistable RipResult either from our cache or ret. a new one. - - The cached RipResult may come from an aborted rip. - - :rtype: :any:`Persistable` for :any:`result.RipResult` - """ - presult = self._pcache.get(cddbdiscid) - - if not presult.object: - logger.debug('result for cddbdiscid %r not in cache', cddbdiscid) - if not create: - logger.debug('returning None') - return None - - logger.debug('creating result') - presult.object = result.RipResult() - presult.persist(presult.object) - else: - logger.debug('result for cddbdiscid %r found in cache, reusing', - cddbdiscid) - - return presult - - def getIds(self): - paths = glob.glob(os.path.join(self._path, '*.pickle')) - - return [os.path.splitext(os.path.basename(path))[0] for path in paths] - - -class TableCache: - """ - Read and write entries to and from the cache of tables. - - If no path is specified, the cache will write to the current cache - directory and read from all possible cache directories (to allow for - pre-0.2.1 cddbdiscid-keyed entries). - """ - - def __init__(self, path=None): - if not path: - self._path = directory.cache_path('table') - else: - self._path = path - - self._pcache = PersistedCache(self._path) - - def get(self, cddbdiscid, mbdiscid): - # Before 0.2.1, we only saved by cddbdiscid, and had collisions - # mbdiscid collisions are a lot less likely - ptable = self._pcache.get('mbdiscid.' + mbdiscid) - - if not ptable.object: - ptable = self._pcache.get(cddbdiscid) - if ptable.object: - if ptable.object.getMusicBrainzDiscId() != mbdiscid: - logger.debug('cached table is for different mb id %r', - ptable.object.getMusicBrainzDiscId()) - ptable.object = None - else: - logger.debug('no valid cached table found for %r', cddbdiscid) - - if not ptable.object: - # get an empty persistable from the writable location - ptable = self._pcache.get('mbdiscid.' + mbdiscid) - - return ptable diff --git a/whipper/common/directory.py b/whipper/common/directory.py index 55b3880d..63901a18 100644 --- a/whipper/common/directory.py +++ b/whipper/common/directory.py @@ -29,15 +29,6 @@ def config_path(): return join(path, 'whipper.conf') -def cache_path(name=None): - path = join(getenv('XDG_CACHE_HOME') or join(expanduser('~'), '.cache'), - 'whipper') - if name: - path = join(path, name) - makedirs(path, exist_ok=True) - return path - - def data_path(name=None): path = join(getenv('XDG_DATA_HOME') or join(expanduser('~'), '.local/share'), diff --git a/whipper/common/program.py b/whipper/common/program.py index c7fa90f1..70884533 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -27,7 +27,7 @@ import time from tempfile import NamedTemporaryFile -from whipper.common import accurip, cache, checksum, common, mbngs, path +from whipper.common import accurip, checksum, common, mbngs, path from whipper.program import cdrdao, cdparanoia from whipper.image import image from whipper.extern import freedb @@ -64,7 +64,6 @@ def __init__(self, config, record=False): :param record: whether to record results of API calls for playback """ self._record = record - self._cache = cache.ResultCache() self._config = config d = {} @@ -113,37 +112,19 @@ def getFastToc(self, runner, device): def getTable(self, runner, cddbdiscid, mbdiscid, device, offset, toc_path): """ - Retrieve the Table either from the cache or the drive. + Retrieve the Table from the drive. :rtype: table.Table """ - tcache = cache.TableCache() - ptable = tcache.get(cddbdiscid, mbdiscid) itable = None tdict = {} - # Ignore old cache, since we do not know what offset it used. - if isinstance(ptable.object, dict): - tdict = ptable.object - - if offset in tdict: - itable = tdict[offset] - - if not itable: - logger.debug('getTable: cddbdiscid %s, mbdiscid %s not in cache ' - 'for offset %s, reading table', cddbdiscid, mbdiscid, - offset) - t = cdrdao.ReadTOCTask(device, toc_path=toc_path) - t.description = "Reading table" - runner.run(t) - itable = t.toc.table - tdict[offset] = itable - ptable.persist(tdict) - logger.debug('getTable: read table %r', itable) - else: - logger.debug('getTable: cddbdiscid %s, mbdiscid %s in cache ' - 'for offset %s', cddbdiscid, mbdiscid, offset) - logger.debug('getTable: loaded table %r', itable) + t = cdrdao.ReadTOCTask(device, toc_path=toc_path) + t.description = "Reading table" + runner.run(t) + itable = t.toc.table + tdict[offset] = itable + logger.debug('getTable: read table %r', itable) assert itable.hasTOC() @@ -153,24 +134,6 @@ def getTable(self, runner, cddbdiscid, mbdiscid, device, offset, itable.getMusicBrainzDiscId()) return itable - def getRipResult(self, cddbdiscid): - """ - Get the persistable RipResult either from our cache or ret. a new one. - - The cached RipResult may come from an aborted rip. - - :rtype: result.RipResult - """ - assert self.result is None - - self._presult = self._cache.getRipResult(cddbdiscid) - self.result = self._presult.object - - return self.result - - def saveRipResult(self): - self._presult.persist() - @staticmethod def addDisambiguation(template_part, metadata): """Add disambiguation to template path part string.""" diff --git a/whipper/test/test_common_accurip.py b/whipper/test/test_common_accurip.py index 4e90af01..32914244 100644 --- a/whipper/test/test_common_accurip.py +++ b/whipper/test/test_common_accurip.py @@ -3,13 +3,9 @@ import sys from io import StringIO -from os import chmod, makedirs -from os.path import dirname, exists, join -from shutil import copy, rmtree -from tempfile import mkdtemp +from os.path import dirname, join from unittest import TestCase -from whipper.common import accurip from whipper.common.accurip import ( calculate_checksums, get_db_entry, print_report, verify_result, _split_responses, EntryNotFound @@ -25,41 +21,10 @@ def setUpClass(cls): cls.entry = _split_responses(f.read()) cls.other_path = '4/8/2/dBAR-011-0010e284-009228a3-9809ff0b.bin' - def setUp(self): - self.cache_dir = mkdtemp(suffix='whipper_accurip_cache_test') - accurip._CACHE_DIR = self.cache_dir - - def cleanup(cachedir): - chmod(cachedir, 0o755) - rmtree(cachedir) - self.addCleanup(cleanup, self.cache_dir) - - def test_uses_cache_dir(self): - # copy normal entry into other entry's place - makedirs(dirname(join(self.cache_dir, self.other_path))) - copy( - join(dirname(__file__), self.path[6:]), - join(self.cache_dir, self.other_path) - ) - # ask cache for other entry and assert cached entry equals normal entry - self.assertEqual(self.entry, get_db_entry(self.other_path)) - def test_raises_entrynotfound_for_no_entry(self): with self.assertRaises(EntryNotFound): get_db_entry('definitely_a_404') - def test_can_return_entry_without_saving(self): - chmod(self.cache_dir, 0) - self.assertEqual(get_db_entry(self.path), self.entry) - chmod(self.cache_dir, 0o755) - self.assertFalse(exists(join(self.cache_dir, self.path))) - - def test_retrieves_and_saves_accuraterip_entry(self): - # for path, entry in zip(self.paths[0], self.entries): - self.assertFalse(exists(join(self.cache_dir, self.path))) - self.assertEqual(get_db_entry(self.path), self.entry) - self.assertTrue(exists(join(self.cache_dir, self.path))) - def test_AccurateRipResponse_parses_correctly(self): responses = get_db_entry(self.path) self.assertEqual(len(responses), 2) diff --git a/whipper/test/test_common_cache.py b/whipper/test/test_common_cache.py deleted file mode 100644 index 44223060..00000000 --- a/whipper/test/test_common_cache.py +++ /dev/null @@ -1,23 +0,0 @@ -# -*- Mode: Python; test-case-name: whipper.test.test_common_cache -*- -# vi:si:et:sw=4:sts=4:ts=4 - -import os - -from whipper.common import cache - -from whipper.test import common as tcommon - - -class ResultCacheTestCase(tcommon.TestCase): - - def setUp(self): - self.cache = cache.ResultCache( - os.path.join(os.path.dirname(__file__), 'cache', 'result')) - - def testGetResult(self): - result = self.cache.getRipResult('fe105a11') - self.assertEqual(result.object.title, "The Writing's on the Wall") - - def testGetIds(self): - ids = self.cache.getIds() - self.assertEqual(ids, ['fe105a11']) diff --git a/whipper/test/test_common_directory.py b/whipper/test/test_common_directory.py index f9b7c26a..c7a8bf24 100644 --- a/whipper/test/test_common_directory.py +++ b/whipper/test/test_common_directory.py @@ -13,6 +13,3 @@ class DirectoryTestCase(common.TestCase): def testAll(self): path = directory.config_path() self.assertTrue(path.startswith(DirectoryTestCase.HOME_PARENT)) - - path = directory.cache_path() - self.assertTrue(path.startswith(DirectoryTestCase.HOME_PARENT))