From 4b689a7a7c27ee8ec1d4dbf41ea7a8a84aa72db9 Mon Sep 17 00:00:00 2001
From: jakedt <jake@devtable.com>
Date: Tue, 25 Feb 2014 16:31:52 -0500
Subject: [PATCH] Fix the translation of old ids to new ids, again.

---
 data/model.py              | 21 ++++++++++++++++-----
 endpoints/index.py         |  6 ++----
 initdb.py                  |  3 ++-
 test/test_image_sharing.py |  2 +-
 4 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/data/model.py b/data/model.py
index 79738ec4e..876f4e0a5 100644
--- a/data/model.py
+++ b/data/model.py
@@ -884,16 +884,20 @@ def create_repository(namespace, name, creating_user, visibility='private'):
   return repo
 
 
-def __translate_ancestry(old_ancestry, translations, existing_images):
+def __translate_ancestry(old_ancestry, translations, repository, username):
   if old_ancestry == '/':
     return '/'
 
   def translate_id(old_id):
+    logger.debug('Translating id: %s', old_id)
     if old_id not in translations:
       # Figure out which docker_image_id the old id refers to, then find a
       # a local one
       old = Image.select(Image.docker_image_id).where(Image.id == old_id).get()
-      translations[old_id] = existing_images[old.docker_image_id]
+      image_in_repo = find_create_or_link_image(old.docker_image_id,
+                                                repository, username,
+                                                translations)
+      translations[old_id] = image_in_repo.id
 
     return translations[old_id]
 
@@ -902,9 +906,14 @@ def __translate_ancestry(old_ancestry, translations, existing_images):
   return '/%s/' % '/'.join(new_ids)
 
 
-def create_or_link_image(docker_image_id, repository, username, translations,
-                         existing_images):
+def find_create_or_link_image(docker_image_id, repository, username,
+                              translations):
   with transaction_factory(db):
+    repo_image = get_repo_image(repository.namespace, repository.name,
+                                docker_image_id)
+    if repo_image:
+      return repo_image
+
     query = (Image
       .select(Image, ImageStorage)
       .distinct()
@@ -926,7 +935,8 @@ def create_or_link_image(docker_image_id, repository, username, translations,
       logger.debug(msg, docker_image_id, to_copy.storage.uuid)
       
       new_image_ancestry = __translate_ancestry(to_copy.ancestors,
-                                                translations, existing_images)
+                                                translations, repository,
+                                                username)
 
       storage = to_copy.storage
       origin_image_id = to_copy.id
@@ -939,6 +949,7 @@ def create_or_link_image(docker_image_id, repository, username, translations,
                              ancestors=new_image_ancestry)
 
     if origin_image_id:
+      logger.debug('Storing translation %s -> %s', origin_image_id, new_image.id)
       translations[origin_image_id] = new_image.id
 
     return new_image
diff --git a/endpoints/index.py b/endpoints/index.py
index e75cb020d..549bcfec8 100644
--- a/endpoints/index.py
+++ b/endpoints/index.py
@@ -193,17 +193,15 @@ def create_repository(namespace, repository):
                               for desc in image_descriptions])
   new_repo_images = dict(added_images)
 
-  existing_image_translations = {}
   for existing in model.get_repository_images(namespace, repository):
     if existing.docker_image_id in new_repo_images:
-      existing_image_translations[existing.docker_image_id] = existing.id
       added_images.pop(existing.docker_image_id)
 
   username = get_authenticated_user() and get_authenticated_user().username
   translations = {}
   for image_description in added_images.values():
-    model.create_or_link_image(image_description['id'], repo, username,
-                               translations, existing_image_translations)
+    model.find_create_or_link_image(image_description['id'], repo, username,
+                                    translations)
 
   response = make_response('Created', 201)
 
diff --git a/initdb.py b/initdb.py
index 87cc31dcf..06ca0053c 100644
--- a/initdb.py
+++ b/initdb.py
@@ -67,7 +67,8 @@ def __create_subtree(repo, structure, creator_username, parent):
     logger.debug('new docker id: %s' % docker_image_id)
     checksum = __gen_checksum(docker_image_id)
 
-    new_image = model.create_or_link_image(docker_image_id, repo, None, {}, {})
+    new_image = model.find_create_or_link_image(docker_image_id, repo, None,
+                                                {})
     new_image.storage.uuid = IMAGE_UUIDS[image_num % len(IMAGE_UUIDS)]
     new_image.storage.save()
 
diff --git a/test/test_image_sharing.py b/test/test_image_sharing.py
index 40e9485cd..bdc85cedf 100644
--- a/test/test_image_sharing.py
+++ b/test/test_image_sharing.py
@@ -43,7 +43,7 @@ class TestImageSharing(unittest.TestCase):
 
   def createStorage(self, docker_image_id, repository=REPO, username=ADMIN_ACCESS_USER):
     repository_obj = model.get_repository(repository.split('/')[0], repository.split('/')[1])
-    image = model.create_or_link_image(docker_image_id, repository_obj, username, {}, {})
+    image = model.find_create_or_link_image(docker_image_id, repository_obj, username, {})
     return image.storage.id
 
   def assertSameStorage(self, docker_image_id, storage_id, repository=REPO, username=ADMIN_ACCESS_USER):