From db32726caee86acff7d3c388bdf680a524b22561 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Wed, 15 Jul 2026 15:19:16 +0300 Subject: [PATCH] remove minimal raw sql metadata endpoint --- addons/osfstorage/tests/test_views.py | 74 +++++++++++---------------- addons/osfstorage/views.py | 66 +++--------------------- 2 files changed, 38 insertions(+), 102 deletions(-) diff --git a/addons/osfstorage/tests/test_views.py b/addons/osfstorage/tests/test_views.py index dadcb840e8a..9aa4b2bec58 100644 --- a/addons/osfstorage/tests/test_views.py +++ b/addons/osfstorage/tests/test_views.py @@ -155,15 +155,13 @@ def test_metadata_not_found_lots_of_slashes(self): @pytest.mark.django_db class TestGetChildrenHook(HookTestCase): - def get_children(self, parent, target=None, *, orm=False, **query_params): + def get_children(self, parent, target=None, **query_params): params = { 'fid': parent._id, 'user_id': self.user._id, 'minimal': 'true', **query_params, } - if orm: - params['orm'] = 'true' return self.send_hook( 'osfstorage_get_children', params, @@ -258,44 +256,33 @@ def test_children_metadata_preprint(self): assert res_date_created == expected_date_created def test_minimal_child_fields(self): - for orm in (False, True): - with self.subTest(orm='orm' if orm else 'sql'): - parent = self.node_settings.get_root().append_folder( - f'minimal-child-fields-{"orm" if orm else "sql"}' - ) - record = self._create_child_file(parent, 'magíc.mp3') - folder = parent.append_folder('nested') - res = self.get_children(parent, orm=orm) - assert res.status_code == 200 - assert len(res.json) == 2 - assert {item['name'] for item in res.json} == {record.name, folder.name} - expected_keys = {'kind', 'name', 'path', 'storage'} - if orm: - expected_keys.add('id') - for item in res.json: - assert set(item.keys()) == expected_keys - if item['kind'] == 'file': - assert item['storage'] == { - 'data': { - 'name': record.name, - 'path': record.versions.first().location_hash, - }, - 'settings': { - storage_settings.WATERBUTLER_RESOURCE: record.versions.first().location[ - storage_settings.WATERBUTLER_RESOURCE - ], - }, - } - else: - assert item['storage'] is None - if item['kind'] == 'file': - assert item['path'] == f'/{record._id}' - if orm: - assert item['id'] == record.id - else: - assert item['path'] == f'/{folder._id}/' - if orm: - assert item['id'] == folder.id + parent = self.node_settings.get_root().append_folder('minimal-child-fields') + record = self._create_child_file(parent, 'magíc.mp3') + folder = parent.append_folder('nested') + res = self.get_children(parent) + assert res.status_code == 200 + assert len(res.json) == 2 + assert {item['name'] for item in res.json} == {record.name, folder.name} + for item in res.json: + assert set(item.keys()) == {'id', 'kind', 'name', 'path', 'storage'} + if item['kind'] == 'file': + assert item['storage'] == { + 'data': { + 'name': record.name, + 'path': record.versions.first().location_hash, + }, + 'settings': { + storage_settings.WATERBUTLER_RESOURCE: record.versions.first().location[ + storage_settings.WATERBUTLER_RESOURCE + ], + }, + } + assert item['path'] == f'/{record._id}' + assert item['id'] == record.id + else: + assert item['storage'] is None + assert item['path'] == f'/{folder._id}/' + assert item['id'] == folder.id def test_minimal_pagination(self): parent = self.node_settings.get_root().append_folder('pagination') @@ -304,12 +291,11 @@ def test_minimal_pagination(self): for name in ('a.mp3', 'b.mp3', 'c.mp3') ] children.sort(key=lambda c: c.id) - first_page = self.get_children(parent, orm=True, limit=2) + first_page = self.get_children(parent, limit=2) assert first_page.status_code == 200 assert [item['id'] for item in first_page.json] == [children[0].id, children[1].id] second_page = self.get_children( parent, - orm=True, limit='2', after=str(children[1].id), ) @@ -320,7 +306,7 @@ def test_minimal_pagination(self): def test_minimal_pagination_after_beyond_last_child(self): parent = self.node_settings.get_root() record = self._create_child_file(parent, 'only.mp3') - res = self.get_children(parent, orm=True, after=str(record.id)) + res = self.get_children(parent, after=str(record.id)) assert res.status_code == 200 assert res.json == [] diff --git a/addons/osfstorage/views.py b/addons/osfstorage/views.py index 6bf3a05a76b..c8494ae6a48 100644 --- a/addons/osfstorage/views.py +++ b/addons/osfstorage/views.py @@ -187,57 +187,7 @@ def osfstorage_get_metadata(file_node, **kwargs): return file_node.serialize(version=version, include_full=True) -# TODO: Remove _osfstorage_minimal_metadata_sql if the Django ORM implementation -# performs well in production benchmarks and zip/DAZ workloads. -def _osfstorage_minimal_metadata_sql(file_node): - waterbutler_resource = osf_storage_settings.WATERBUTLER_RESOURCE - with connection.cursor() as cursor: - cursor.execute(""" - SELECT json_agg( - json_build_object( - 'kind', CASE - WHEN F.type = 'osf.osfstoragefile' THEN 'file' - ELSE 'folder' - END, - 'name', F.name, - 'path', CASE - WHEN F.type = 'osf.osfstoragefile' THEN '/' || F._id - ELSE '/' || F._id || '/' - END, - 'storage', CASE - WHEN F.type = 'osf.osfstoragefile' THEN - json_build_object( - 'data', json_build_object( - 'name', COALESCE(LATEST_VERSION.version_name, F.name), - 'path', LATEST_VERSION.location ->> 'object' - ), - 'settings', json_build_object( - %s, LATEST_VERSION.location ->> %s - ) - ) - ELSE NULL - END - ) - ) - FROM osf_basefilenode AS F - LEFT JOIN LATERAL ( - SELECT - osf_fileversion.location, - osf_basefileversionsthrough.version_name - FROM osf_fileversion - JOIN osf_basefileversionsthrough - ON osf_fileversion.id = osf_basefileversionsthrough.fileversion_id - WHERE osf_basefileversionsthrough.basefilenode_id = F.id - ORDER BY osf_fileversion.created DESC - LIMIT 1 - ) LATEST_VERSION ON F.type = 'osf.osfstoragefile' - WHERE F.parent_id = %s - AND F.type IN ('osf.osfstoragefile', 'osf.osfstoragefolder') - """, [waterbutler_resource, waterbutler_resource, file_node.id]) - return cursor.fetchone()[0] or [] - - -def _osfstorage_minimal_metadata_orm(file_node, *, limit=None, after=None): +def _osfstorage_minimal_metadata(file_node, *, limit=None, after=None): waterbutler_resource = osf_storage_settings.WATERBUTLER_RESOURCE latest_version = BaseFileVersionsThrough.objects.filter( basefilenode_id=OuterRef('pk'), @@ -283,6 +233,8 @@ def _osfstorage_minimal_metadata_orm(file_node, *, limit=None, after=None): if after is not None: qs = qs.filter(id__gt=after) + # Always return files/folders in the same order + # It helps make waterbutler file streaming more stable qs = qs.order_by('id') if limit is not None: @@ -402,13 +354,11 @@ def _osfstorage_full_metadata(file_node, user_id): @decorators.autoload_filenode(must_be='folder') def osfstorage_get_children(file_node, **kwargs): if is_truthy(request.args.get('minimal')): - if is_truthy(request.args.get('orm')): - return _osfstorage_minimal_metadata_orm( - file_node, - limit=request.args.get('limit', type=int, default=None), - after=request.args.get('after', type=int, default=None), - ) - return _osfstorage_minimal_metadata_sql(file_node) + return _osfstorage_minimal_metadata( + file_node, + limit=request.args.get('limit', type=int, default=None), + after=request.args.get('after', type=int, default=None), + ) return _osfstorage_full_metadata(file_node, request.args.get('user_id'))