From e848482ca1fd44c1cdeedb07966c4ddb60488b2a Mon Sep 17 00:00:00 2001 From: Tim Gion Date: Mon, 24 Apr 2017 10:50:47 -0400 Subject: [PATCH] Fix for lookup table corruption. Issue occurred when the same experiment name used in different collections. When one of those experiments renamed, updates meant to rename the experiment referenced by child channels also updated the experiments with the same name, in the lookup table. Lookup table query now also filters on the collection name to ensure updates are isolated to the renamed experiment's channels. --- django/bosscore/lookup.py | 5 ++- django/bosscore/test/setup_db.py | 28 +++++++++++++- django/bosscore/test/test_lookup.py | 57 +++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 django/bosscore/test/test_lookup.py diff --git a/django/bosscore/lookup.py b/django/bosscore/lookup.py index 98fd1a4b..3e86e878 100644 --- a/django/bosscore/lookup.py +++ b/django/bosscore/lookup.py @@ -198,8 +198,9 @@ def update_lookup_experiment(lookup_key, boss_key, collection_name, experiment_n # update all channels that reference this experiment - all_lookup_objs = BossLookup.objects.filter(experiment_name=old_experiment_name).exclude( - lookup_key=lookup_key) + all_lookup_objs = BossLookup.objects.filter( + collection_name=collection_name, experiment_name=old_experiment_name).exclude( + lookup_key=lookup_key) for item in all_lookup_objs: split_key = item.boss_key.split('&') diff --git a/django/bosscore/test/setup_db.py b/django/bosscore/test/setup_db.py index 5771e793..8bbd4d82 100644 --- a/django/bosscore/test/setup_db.py +++ b/django/bosscore/test/setup_db.py @@ -91,15 +91,41 @@ def create_group(self, group_name): def insert_test_data(self): self.add_collection('col1', 'Description for collection1') - self.add_collection('col1-22', 'Description for collection1') + self.add_collection('col1-22', 'Description for collection1-22') self.add_collection('col2', 'Description for collection2') + self.add_coordinate_frame('cf1', 'Description for cf1', 0, 1000, 0, 1000, 0, 1000, 4, 4, 4) + self.add_experiment('col1', 'exp1', 'cf1', 10, 10, 1) self.add_experiment('col1', 'exp22', 'cf1', 10, 500, 1) + + self.add_channel('col1', 'exp1', 'channel1', 0, 0, 'uint8', 'image') + self.add_channel('col1', 'exp1', 'channel2', 0, 0, 'uint8', 'image') + self.add_channel('col1', 'exp1', 'channel3', 0, 0, 'uint64', 'annotation', ['channel1']) + self.add_channel('col1', 'exp1', 'layer1', 0, 0, 'uint64', 'annotation', ['channel1']) + + def insert_lookup_test_data(self): + """ + Test data for LookupTest test case. + """ + + self.add_collection('col1', 'Description for collection1') + self.add_collection('col2', 'Description for collection2') + + self.add_coordinate_frame('cf1', 'Description for cf1', 0, 1000, 0, 1000, 0, 1000, 4, 4, 4) + + self.add_experiment('col1', 'exp1', 'cf1', 10, 10, 1) + + # This experiment is _purposed_ named the same as the exp in col1. + # Ensuring that renaming an experiment does not affect experiments with + # the same name in other collections. + self.add_experiment('col2', 'exp1', 'cf1', 10, 500, 1) + self.add_channel('col1', 'exp1', 'channel1', 0, 0, 'uint8', 'image') self.add_channel('col1', 'exp1', 'channel2', 0, 0, 'uint8', 'image') self.add_channel('col1', 'exp1', 'channel3', 0, 0, 'uint64', 'annotation', ['channel1']) self.add_channel('col1', 'exp1', 'layer1', 0, 0, 'uint64', 'annotation', ['channel1']) + self.add_channel('col2', 'exp1', 'channel1', 0, 0, 'uinit8', 'image') def insert_spatialdb_test_data(self): diff --git a/django/bosscore/test/test_lookup.py b/django/bosscore/test/test_lookup.py new file mode 100644 index 00000000..8935cabe --- /dev/null +++ b/django/bosscore/test/test_lookup.py @@ -0,0 +1,57 @@ +# Copyright 2016 The Johns Hopkins University Applied Physics Laboratory +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from bosscore.lookup import LookUpKey +from bosscore.models import Collection, Experiment, Channel +from bosscore.models import BossLookup +from rest_framework.test import APITestCase +from .setup_db import SetupTestDB +import unittest + + +class LookupTest(APITestCase): + def setUp(self): + dbsetup = SetupTestDB() + user = dbsetup.create_user('testuser') + dbsetup.add_role('resource-manager') + dbsetup.set_user(user) + + self.client.force_login(user) + dbsetup.insert_lookup_test_data() + + def test_update_lookup_experiment(self): + """ + On an experiment rename, make sure only resources that are children of + the experiment are changed. + + This is a regression test. + """ + new_exp_name = 'new_exp' + collection = 'col2' + orig_exp_name = 'exp1' + + collection_obj = Collection.objects.get(name=collection) + experiment_obj = Experiment.objects.get(name=orig_exp_name, collection=collection_obj) + lookup_key = str(collection_obj.pk) + '&' + str(experiment_obj.pk) + boss_key = collection_obj.name + '&' + new_exp_name + + # Method under test. + LookUpKey.update_lookup_experiment( + lookup_key, boss_key, collection_obj.name, new_exp_name) + + # There should be still 5 rows in the lookup table with orig_exp_name + # as the experiment name under col1. There should be the experiment + # and 4 channels. + all_lookup_objs = BossLookup.objects.filter(experiment_name=orig_exp_name) + self.assertEqual(5, len(all_lookup_objs))