Skip to content

Commit

Permalink
improve tests for gravyvalet feature flagging
Browse files Browse the repository at this point in the history
  • Loading branch information
John Tordoff committed May 21, 2024
1 parent bc3fd85 commit e888e87
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 17 deletions.
1 change: 0 additions & 1 deletion addons/base/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ def __init__(self, resource, auth, legacy_config):

@staticmethod
def get_configured_storage_addons_data(config_id, auth):
print(auth)
resp = requests.get(
settings.GV_NODE_ADDON_ENDPOINT.format(config_id=config_id),
)
Expand Down
13 changes: 10 additions & 3 deletions api/base/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,10 +635,13 @@ def bulk_get_file_nodes_from_wb_resp(self, files_list):

for item in files_list:
attrs = item['attributes']
config = GravyValetAddonAppConfig(self, attrs['provider'], self.request)
if waffle.flag_is_active(self.request, features.ENABLE_GV):
short_name = GravyValetAddonAppConfig(self, attrs['provider'], self.request).legacy_config.short_name
else:
short_name = attrs['provider']

base_class = BaseFileNode.resolve_class(
config.legacy_config.short_name,
short_name,
BaseFileNode.FOLDER if attrs['kind'] == 'folder'
else BaseFileNode.FILE,
)
Expand All @@ -665,7 +668,11 @@ def bulk_get_file_nodes_from_wb_resp(self, files_list):
else:
file_objs.append(file_obj)

file_obj.provider = config.config_id
# TODO: Improve robustness
if waffle.flag_is_active(self.request, features.ENABLE_GV):
config = GravyValetAddonAppConfig(self, attrs['provider'], self.request).legacy_config
file_obj.provider = config.config_id

file_obj.update(None, attrs, user=self.request.user, save=False)

bulk_update(file_objs)
Expand Down
2 changes: 1 addition & 1 deletion api/nodes/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1452,7 +1452,7 @@ class Meta:
def get_id(obj):
from osf.utils.requests import get_current_request
if waffle.flag_is_active(get_current_request(), features.ENABLE_GV):
return obj.id
return obj.provider
else:
return f'{obj.node._id}:{obj.provider}'

Expand Down
2 changes: 1 addition & 1 deletion api/nodes/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def get_file_object(target, path, provider, request):
headers={'Authorization': request.META.get('HTTP_AUTHORIZATION')},
)

if waterbutler_request.status_code == 401:
if waterbutler_request.status_code in (401, 403):
raise PermissionDenied

if waterbutler_request.status_code == 404:
Expand Down
43 changes: 32 additions & 11 deletions api_tests/providers/test_files_with_gv.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
)
from waffle.testutils import override_flag
from django.shortcuts import reverse
from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response


@pytest.mark.django_db
class TestWaffleFilesProviderView:
"""
Just passing id as name, no need to mock GV
"""

@pytest.fixture(autouse=True)
def override_flag(self):
Expand All @@ -31,7 +35,6 @@ def provider_gv_id(self):
def node(self, user):
return ProjectFactory(
creator=user,
comment_level='public'
)

@pytest.fixture()
Expand Down Expand Up @@ -88,17 +91,13 @@ def test_must_be_contributor(self, app, addon_files_url):
assert res.status_code == 403

def test_get_file_provider(self, app, user, addon_files_url, file, provider_gv_id):

res = app.get(
addon_files_url,
auth=user.auth
)
res = app.get(addon_files_url, auth=user.auth)

assert res.status_code == 200
attributes = res.json['data']['attributes']
assert attributes['provider'] == str(provider_gv_id)
assert attributes['name'] == str(provider_gv_id)
assert res.json['data']['id'] == provider_gv_id
assert res.json['data']['id'] == str(provider_gv_id)


@pytest.mark.django_db
Expand Down Expand Up @@ -146,14 +145,38 @@ def file_url(self, node, provider_gv_id, file):
}
)

def test_must_have_auth(self, app, file_url):
@responses.activate
def test_must_have_auth(self, app, user, file_url, file, provider_gv_id, node):
prepare_mock_wb_response(
node=node,
path=file.path + '/',
provider='1',
status_code=505 # invalid auth should not reach mock
)

responses.add_passthru('http://192.168.168.167:8004/v1/configured-storage-addons/1/base_account/')
responses.add_passthru('http://192.168.168.167:8004/v1/authorized-storage-accounts/2/external_storage_service/')

res = app.get(
file_url,
auth=('invaid', 'auth'),
expect_errors=True
)
assert res.status_code != 505 # invalid auth should not reach mock response
assert res.status_code == 401

def test_must_be_contributor(self, app, file_url):
@responses.activate
def test_must_be_contributor(self, app, user, file_url, file, provider_gv_id, node):
prepare_mock_wb_response(
node=node,
path=file.path + '/',
provider='1',
status_code=403
)

responses.add_passthru('http://192.168.168.167:8004/v1/configured-storage-addons/1/base_account/')
responses.add_passthru('http://192.168.168.167:8004/v1/authorized-storage-accounts/2/external_storage_service/')

res = app.get(
file_url,
auth=AuthUserFactory().auth,
Expand All @@ -163,8 +186,6 @@ def test_must_be_contributor(self, app, file_url):

@responses.activate
def test_get_file_provider(self, app, user, file_url, file, provider_gv_id, node):
from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response

prepare_mock_wb_response(
path=file.path + '/',
node=node,
Expand Down

0 comments on commit e888e87

Please sign in to comment.