From a65e6811fed4529789f3c72e2f4d56b81fb71526 Mon Sep 17 00:00:00 2001 From: Marcin Koziej Date: Thu, 17 May 2012 09:48:56 +0200 Subject: [PATCH] Move gallery merging logic to a helper class, to keep views as clean as possible. Check for last-first duplicate Tests --- apps/catalogue/helpers.py | 107 +++++++++++++++++++++++++++++++ apps/catalogue/models/book.py | 85 ++---------------------- apps/catalogue/tests/__init__.py | 9 +-- redakcja/settings/test.py | 3 + 4 files changed, 122 insertions(+), 82 deletions(-) diff --git a/apps/catalogue/helpers.py b/apps/catalogue/helpers.py index df64ade1..a97a0400 100644 --- a/apps/catalogue/helpers.py +++ b/apps/catalogue/helpers.py @@ -1,5 +1,11 @@ from datetime import date from functools import wraps +from os.path import join +from os import listdir, stat +from shutil import move, rmtree +from django.conf import settings +import re +import filecmp from django.db.models import Count @@ -36,3 +42,104 @@ def parse_isodate(isodate): return date(*[int(p) for p in isodate.split('-')]) except (AttributeError, TypeError, ValueError): raise ValueError("Not a date in ISO format.") + + +class GalleryMerger(object): + def __init__(self, dest_gallery, src_gallery): + assert isinstance(dest_gallery, str) + assert isinstance(src_gallery, str) + self.dest = dest_gallery + self.src = src_gallery + self.dest_size = None + self.src_size = None + self.num_deleted = 0 + + @staticmethod + def path(gallery): + return join(settings.MEDIA_ROOT, settings.IMAGE_DIR, gallery) + + @staticmethod + def get_prefix(name): + m = re.match(r"^([0-9])-", name) + if m: + return int(m.groups()[0]) + return None + + @staticmethod + def set_prefix(name, prefix, always=False): + m = not always and re.match(r"^([0-9])-", name) + return "%1d-%s" % (prefix, m and name[2:] or name) + + def merge(self): + if not self.dest: + return self.src + if not self.src: + return self.dest + + files = listdir(self.path(self.dest)) + self.dest_size = len(files) + files_other = listdir(self.path(self.src)) + self.src_size = len(files_other) + + if files and files_other: + print "compare %s with %s" % (files[-1], files_other[0]) + if filecmp.cmp( + join(self.path(self.dest), files[-1]), + join(self.path(self.src), files_other[0]), + False + ): + files_other.pop(0) + self.num_deleted = 1 + + prefixes = {} + renamed_files = {} + renamed_files_other = {} + last_pfx = -1 + + # check if all elements of my files have a prefix + files_prefixed = True + for f in files: + p = self.get_prefix(f) + if p: + if p > last_pfx: last_pfx = p + else: + files_prefixed = False + break + + # if not, add a 0 prefix to them + if not files_prefixed: + prefixes[0] = 0 + for f in files: + renamed_files[f] = self.set_prefix(f, 0, True) + + # two cases here - either all are prefixed or not. + files_other_prefixed = True + for f in files_other: + pfx = self.get_prefix(f) + if pfx is not None: + if not pfx in prefixes: + last_pfx += 1 + prefixes[pfx] = last_pfx + renamed_files_other[f] = self.set_prefix(f, prefixes[pfx]) + else: + # ops, not all files here were prefixed. + files_other_prefixed = False + break + + # just set a 1- prefix to all of them + if not files_other_prefixed: + for f in files_other: + renamed_files_other[f] = self.set_prefix(f, 1, True) + + # finally, move / rename files. + from nose.tools import set_trace + # set_trace() + for frm, to in renamed_files.items(): + move(join(self.path(self.dest), frm), + join(self.path(self.dest), to)) + for frm, to in renamed_files_other.items(): + move(join(self.path(self.src), frm), + join(self.path(self.dest), to)) + + rmtree(join(self.path(self.src))) + return self.dest diff --git a/apps/catalogue/models/book.py b/apps/catalogue/models/book.py index 098781dd..df1ff4e0 100755 --- a/apps/catalogue/models/book.py +++ b/apps/catalogue/models/book.py @@ -12,7 +12,7 @@ from slughifi import slughifi import apiclient -from catalogue.helpers import cached_in_field +from catalogue.helpers import cached_in_field, GalleryMerger from catalogue.models import BookPublishRecord, ChunkPublishRecord from catalogue.signals import post_publish from catalogue.tasks import refresh_instance, book_content_updated @@ -196,87 +196,16 @@ class Book(models.Model): number += 1 assert not other.chunk_set.exists() - self.append_gallery(other, len_other) - - other.delete() - - - def append_gallery(self, other, len_other): - if self.gallery is None: - self.gallery = other.gallery - return - if other.gallery is None: - return - - def get_prefix(name): - m = re.match(r"^([0-9])-", name) - if m: - return int(m.groups()[0]) - return None - - def set_prefix(name, prefix, always=False): - m = not always and re.match(r"^([0-9])-", name) - return "%1d-%s" % (prefix, m and name[2:] or name) - - files = os.listdir(os.path.join(settings.MEDIA_ROOT, - settings.IMAGE_DIR, self.gallery)) - files_other = os.listdir(os.path.join(settings.MEDIA_ROOT, - settings.IMAGE_DIR, other.gallery)) - - prefixes = {} - renamed_files = {} - renamed_files_other = {} - last_pfx = -1 - - # check if all elements of my files have a prefix - files_prefixed = True - for f in files: - p = get_prefix(f) - if p: - if p > last_pfx: last_pfx = p - else: - files_prefixed = False - break - - # if not, add a 0 prefix to them - if not files_prefixed: - prefixes[0] = 0 - for f in files: - renamed_files[f] = set_prefix(f, 0, True) - - # two cases here - either all are prefixed or not. - files_other_prefixed = True - for f in files_other: - pfx = get_prefix(f) - if pfx is not None: - if not pfx in prefixes: - last_pfx += 1 - prefixes[pfx] = last_pfx - renamed_files_other[f] = set_prefix(f, prefixes[pfx]) - else: - # ops, not all files here were prefixed. - files_other_prefixed = False - break - - # just set a 1- prefix to all of them - if not files_other_prefixed: - for f in files_other: - renamed_files_other[f] = set_prefix(f, 1, True) - - # finally, move / rename files. - for frm, to in renamed_files.items(): - shutil.move(os.path.join(settings.MEDIA_ROOT, settings.IMAGE_DIR, self.gallery, frm), - os.path.join(settings.MEDIA_ROOT, settings.IMAGE_DIR, self.gallery, to)) - for frm, to in renamed_files_other.items(): - shutil.move(os.path.join(settings.MEDIA_ROOT, settings.IMAGE_DIR, other.gallery, frm), - os.path.join(settings.MEDIA_ROOT, settings.IMAGE_DIR, self.gallery, to)) + gm = GalleryMerger(self.gallery, other.gallery) + self.gallery = gm.merge() # and move the gallery starts - num_files = len(files) + num_files = gm.dest_size for chunk in self[len(self) - len_other:]: - chunk.gallery_start += num_files + chunk.gallery_start += gm.dest_size - gm.num_deleted chunk.save() - + + other.delete() @transaction.commit_on_success diff --git a/apps/catalogue/tests/__init__.py b/apps/catalogue/tests/__init__.py index 91c3f812..ac9298cd 100755 --- a/apps/catalogue/tests/__init__.py +++ b/apps/catalogue/tests/__init__.py @@ -118,7 +118,7 @@ class GalleryAppendTests(TestCase): c.gallery_start = 0 c = self.book2[1] c.gallery_start = 2 - import nose.tools; nose.tools.set_trace() + self.make_gallery(self.book1, { '1-0001_1l' : 'aa', '1-0001_2r' : 'bb', @@ -127,7 +127,7 @@ class GalleryAppendTests(TestCase): }) self.make_gallery(self.book2, { - '1-0001_1l' : 'ee', + '1-0001_1l' : 'dd', # the same, should not be moved '1-0001_2r' : 'ff', '2-0002_1l' : 'gg', '2-0002_2r' : 'hh', @@ -141,13 +141,13 @@ class GalleryAppendTests(TestCase): '1-0001_2r', '1-0002_1l', '1-0002_2r', - '2-0001_1l', + # '2-0001_1l', '2-0001_2r', '3-0002_1l', '3-0002_2r', ]) - self.assertEqual((4, 6), (self.book1[2].gallery_start, self.book1[3].gallery_start)) + self.assertEqual((3, 5), (self.book1[2].gallery_start, self.book1[3].gallery_start)) def test_none_indexed(self): @@ -183,6 +183,7 @@ class GalleryAppendTests(TestCase): def test_none_indexed(self): + import nose.tools self.book2 = Book.create(self.user, 'book 2', slug='book2') self.make_gallery(self.book1, { '1-0001_1l' : 'aa', diff --git a/redakcja/settings/test.py b/redakcja/settings/test.py index 5fbd59f5..dcbbb1f9 100644 --- a/redakcja/settings/test.py +++ b/redakcja/settings/test.py @@ -20,11 +20,14 @@ DATABASES = { import tempfile CATALOGUE_REPO_PATH = tempfile.mkdtemp(prefix='redakcja-repo') +MEDIA_ROOT = tempfile.mkdtemp(prefix='media-root') USE_CELERY = False INSTALLED_APPS += ('django_nose', 'dvcs.tests') TEST_RUNNER = 'django_nose.NoseTestSuiteRunner' +#TEST_MODULES = ('catalogue') + TEST_MODULES = ('catalogue', 'dvcs.tests', 'wiki', 'toolbar') COVER_APPS = ('catalogue', 'dvcs', 'wiki', 'toolbar') NOSE_ARGS = ( -- 2.20.1