Skip to content

Commit 08ded36

Browse files
zamuzakkizamuzakki
authored
Handle invalid common layers (#68)
* Fix default layer size * Check for invalid common layers after downloaded * Delete invalid default input layers * Add tests * Add more tests
1 parent 8798239 commit 08ded36

File tree

2 files changed

+193
-14
lines changed

2 files changed

+193
-14
lines changed

‎django_project/cplus_api/tasks/sync_default_layers.py

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import tempfile
55

66
import rasterio
7+
from rasterio.errors import RasterioIOError
78
from datetime import datetime
89
from django.utils import timezone
910

@@ -12,6 +13,7 @@
1213
from storages.backends.s3 import S3Storage
1314
from django.conf import settings
1415
from django.core.files.storage import FileSystemStorage
16+
from django.db.models import Q
1517

1618
from cplus_api.models import (
1719
select_input_layer_storage,
@@ -122,18 +124,57 @@ def run(self):
122124
if isinstance(self.storage, FileSystemStorage):
123125
download_path = os.path.join(media_root, self.file['Key'])
124126
os.makedirs(os.path.dirname(download_path), exist_ok=True)
125-
self.read_metadata(download_path)
126-
os.remove(download_path)
127+
iteration = 0
128+
while iteration < 3:
129+
try:
130+
self.read_metadata(download_path)
131+
except RasterioIOError:
132+
iteration += 1
133+
if iteration == 3 and (
134+
self.input_layer.name == '' or
135+
self.input_layer.file is None
136+
):
137+
self.input_layer.delete()
138+
else:
139+
os.remove(download_path)
140+
break
127141
else:
128-
with tempfile.NamedTemporaryFile() as tmpfile:
129-
boto3_client = self.storage.connection.meta.client
130-
boto3_client.download_file(
131-
self.storage.bucket_name,
132-
self.file['Key'],
133-
tmpfile.name,
134-
Config=settings.AWS_TRANSFER_CONFIG
135-
)
136-
self.read_metadata(tmpfile.name)
142+
iteration = 0
143+
while iteration < 3:
144+
with tempfile.NamedTemporaryFile() as tmpfile:
145+
boto3_client = self.storage.connection.meta.client
146+
boto3_client.download_file(
147+
self.storage.bucket_name,
148+
self.file['Key'],
149+
tmpfile.name,
150+
Config=settings.AWS_TRANSFER_CONFIG
151+
)
152+
try:
153+
self.read_metadata(tmpfile.name)
154+
except RasterioIOError:
155+
iteration += 1
156+
if iteration == 3 and (
157+
self.input_layer.name == '' or
158+
self.input_layer.file is None
159+
):
160+
self.input_layer.delete()
161+
else:
162+
break
163+
164+
165+
def delete_invalid_default_layers():
166+
"""Delete invalid default layers in DB
167+
168+
:return: None
169+
:rtype: None
170+
"""
171+
common_layers = InputLayer.objects.filter(
172+
privacy_type=InputLayer.PrivacyTypes.COMMON
173+
)
174+
invalid_common_layers = common_layers.filter(
175+
Q(name='') | Q(file='')
176+
)
177+
invalid_common_layers.delete()
137178

138179

139180
@shared_task(name="sync_default_layers")
@@ -142,6 +183,8 @@ def sync_default_layers():
142183
Create Input Layers from default layers copied to S3/local directory
143184
"""
144185

186+
delete_invalid_default_layers()
187+
145188
storage = select_input_layer_storage()
146189
component_types = [c[0] for c in InputLayer.ComponentTypes.choices]
147190
admin_username = os.getenv('ADMIN_USERNAME')

‎django_project/cplus_api/tests/test_sync_default_layers.py

Lines changed: 139 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,30 @@
11
import os
2+
import tempfile
3+
import time
4+
from datetime import timedelta
25
from shutil import copyfile
6+
from unittest.mock import patch, MagicMock
7+
8+
from django.utils import timezone
9+
from rasterio.errors import RasterioIOError
10+
from storages.backends.s3 import S3Storage
311

412
from core.settings.utils import absolute_path
513
from cplus_api.models.layer import (
614
InputLayer,
715
COMMON_LAYERS_DIR
816
)
9-
from cplus_api.tasks.sync_default_layers import sync_default_layers
17+
from cplus_api.tasks.sync_default_layers import (
18+
sync_default_layers,
19+
ProcessFile
20+
)
1021
from cplus_api.tests.common import BaseAPIViewTransactionTest
22+
from cplus_api.tests.factories import InputLayerF
1123

1224

1325
class TestSyncDefaultLayer(BaseAPIViewTransactionTest):
1426
def setUp(self, *args, **kwargs):
1527
super().setUp(*args, **kwargs)
16-
# print(help(self))
17-
# breakpoint()
1828
self.superuser.username = os.getenv('ADMIN_USERNAME')
1929
self.superuser.save()
2030

@@ -73,6 +83,7 @@ def test_new_layer(self):
7383

7484
def test_file_updated(self):
7585
input_layer, source_path, dest_path = self.base_run()
86+
time.sleep(5)
7687
first_modified_on = input_layer.modified_on
7788
copyfile(source_path, dest_path)
7889
sync_default_layers()
@@ -87,3 +98,128 @@ def test_file_updated(self):
8798
input_layer.refresh_from_db()
8899
self.assertEqual(input_layer.name, 'New Name')
89100
self.assertEqual(input_layer.description, 'New Description')
101+
102+
def test_delete_invalid_layers(self):
103+
input_layer, source_path, dest_path = self.base_run()
104+
invalid_common_layer_1 = InputLayerF.create(
105+
name='',
106+
privacy_type=InputLayer.PrivacyTypes.COMMON,
107+
file=input_layer.file
108+
)
109+
invalid_common_layer_2 = InputLayerF.create(
110+
name='invalid_common_layer_2',
111+
privacy_type=InputLayer.PrivacyTypes.COMMON,
112+
file=None
113+
)
114+
private_layer_1 = InputLayerF.create(
115+
name='',
116+
privacy_type=InputLayer.PrivacyTypes.PRIVATE,
117+
file=input_layer.file
118+
)
119+
private_layer_2 = InputLayerF.create(
120+
name='private_layer_2',
121+
privacy_type=InputLayer.PrivacyTypes.PRIVATE
122+
)
123+
124+
sync_default_layers()
125+
126+
# Calling refresh_from_db() on these 2 variable would result
127+
# in InputLayer.DoesNotExist as they have been deleted,
128+
# because they are invalid common layers
129+
with self.assertRaises(InputLayer.DoesNotExist):
130+
invalid_common_layer_1.refresh_from_db()
131+
with self.assertRaises(InputLayer.DoesNotExist):
132+
invalid_common_layer_2.refresh_from_db()
133+
134+
# These layers are not deleted, so we could still call refresh_from_db
135+
private_layer_1.refresh_from_db()
136+
private_layer_2.refresh_from_db()
137+
138+
def test_invalid_input_layers_not_created(self):
139+
source_path = absolute_path(
140+
'cplus_api', 'tests', 'data',
141+
'pathways', 'test_pathway_2.tif'
142+
)
143+
dest_path = (
144+
f'/home/web/media/minio_test/{COMMON_LAYERS_DIR}/'
145+
f'{InputLayer.ComponentTypes.NCS_PATHWAY}/test_pathway_2.tif'
146+
)
147+
os.makedirs(os.path.dirname(dest_path), exist_ok=True)
148+
copyfile(source_path, dest_path)
149+
with patch.object(
150+
ProcessFile, 'read_metadata', autospec=True
151+
) as mock_read_metadata:
152+
mock_read_metadata.side_effect = [
153+
RasterioIOError('error'),
154+
RasterioIOError('error'),
155+
RasterioIOError('error')
156+
]
157+
sync_default_layers()
158+
159+
self.assertFalse(InputLayer.objects.exists())
160+
161+
def test_invalid_input_layers_not_deleted(self):
162+
input_layer, source_path, dest_path = self.base_run()
163+
time.sleep(5)
164+
first_modified_on = input_layer.modified_on
165+
copyfile(source_path, dest_path)
166+
sync_default_layers()
167+
with patch.object(
168+
ProcessFile, 'read_metadata', autospec=True
169+
) as mock_read_metadata:
170+
mock_read_metadata.side_effect = [
171+
RasterioIOError('error'),
172+
RasterioIOError('error'),
173+
RasterioIOError('error')
174+
]
175+
sync_default_layers()
176+
177+
self.assertTrue(InputLayer.objects.exists())
178+
179+
# Check modified_on is updated
180+
input_layer.refresh_from_db()
181+
self.assertNotEquals(input_layer.modified_on, first_modified_on)
182+
183+
def run_s3(self, mock_storage, mock_named_tmp_file=None):
184+
source_path = absolute_path(
185+
'cplus_api', 'tests', 'data',
186+
'pathways', 'test_pathway_2.tif'
187+
)
188+
dest_path = (
189+
f'/home/web/media/minio_test/{COMMON_LAYERS_DIR}/'
190+
f'{InputLayer.ComponentTypes.NCS_PATHWAY}/test_pathway_2.tif'
191+
)
192+
os.makedirs(os.path.dirname(dest_path), exist_ok=True)
193+
copyfile(source_path, dest_path)
194+
195+
storage = S3Storage(bucket_name='test-bucket')
196+
s3_client = MagicMock()
197+
s3_client.list_objects.return_value = {
198+
'Contents': [
199+
{
200+
'Key': 'common_layers/ncs_pathway/test_pathway_2.tif',
201+
'LastModified': timezone.now() + timedelta(days=1)
202+
}
203+
]
204+
}
205+
storage.connection.meta.client = s3_client
206+
mock_storage.return_value = storage
207+
if mock_named_tmp_file:
208+
(mock_named_tmp_file.return_value.
209+
__enter__.return_value).name = dest_path
210+
sync_default_layers()
211+
212+
@patch('cplus_api.tasks.sync_default_layers.select_input_layer_storage')
213+
def test_invalid_input_layers_not_created_s3(self, mock_storage):
214+
self.run_s3(mock_storage)
215+
self.assertFalse(InputLayer.objects.exists())
216+
217+
@patch('cplus_api.tasks.sync_default_layers.select_input_layer_storage')
218+
@patch.object(tempfile, 'NamedTemporaryFile')
219+
def test_invalid_input_layers_created_s3(
220+
self,
221+
mock_named_tmp_file,
222+
mock_storage
223+
):
224+
self.run_s3(mock_storage, mock_named_tmp_file)
225+
self.assertTrue(InputLayer.objects.exists())

0 commit comments

Comments
 (0)