Skip to content

Commit

Permalink
Fix part of oppia#5002 remove GCS_RESOURCE_BUCKET_NAME from GLOBALS (o…
Browse files Browse the repository at this point in the history 
…ppia#7130)

* remove GCS_RESOURCE_BUCKET_NAME from GLOBALS

* refactorings

* refactor test

* work on failing FE test

* still on failing test

* address review comments

* address review comment

* remove unneeded initialize

* address review comments

* work on erroring backend test

* work on failing tests

* address review comments and made the service initialize itself

* address review comments

* working on FE test

* address review comments

* address review comments and work on failing backend test
  • Loading branch information
@jameesjohn @seanlip
jameesjohn authored and seanlip committed Jul 25, 2019
1 parent 34497a5 commit a7bb506
Show file tree
Hide file tree
Showing 19 changed files with 211 additions and 123 deletions.
3 changes: 0 additions & 3 deletions core/controllers/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
from google.appengine.api import users
import webapp2

app_identity_services = models.Registry.import_app_identity_services()
current_user_services = models.Registry.import_current_user_services()
(user_models,) = models.Registry.import_models([models.NAMES.user])

Expand Down Expand Up @@ -305,8 +304,6 @@ def render_template(self, filepath, iframe_restriction='DENY'):
rights_manager.ACTIVITY_STATUS_PRIVATE),
'ACTIVITY_STATUS_PUBLIC': (
rights_manager.ACTIVITY_STATUS_PUBLIC),
'GCS_RESOURCE_BUCKET_NAME': (
app_identity_services.get_gcs_resource_bucket_name()),
# The 'path' variable starts with a forward slash.
'FULL_URL': '%s://%s%s' % (scheme, netloc, path),
})
Expand Down
16 changes: 16 additions & 0 deletions core/controllers/resources.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from core.domain import config_domain
from core.domain import fs_domain
from core.domain import value_generators_domain
from core.platform import models
import feconf


Expand Down Expand Up @@ -104,3 +105,18 @@ def get(self):
'promo_bar_enabled': config_domain.PROMO_BAR_ENABLED.value,
'promo_bar_message': config_domain.PROMO_BAR_MESSAGE.value
})


class GcsResourceBucketNameHandler(base.BaseHandler):
"""Provides GCS resouce bucket name."""

GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON
app_identity_services = models.Registry.import_app_identity_services()

@acl_decorators.open_access
def get(self):
"""Handles GET requests."""
self.render_json({
'GCS_RESOURCE_BUCKET_NAME': (
self.app_identity_services.get_gcs_resource_bucket_name()),
})
35 changes: 35 additions & 0 deletions core/controllers/resources_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,3 +606,38 @@ def test_non_matching_extensions_are_detected(self):
self.assertEqual(response_dict['status_code'], 400)
self.assertEqual(response_dict['error'], 'Audio not recognized as '
'a mp3 file')


class GcsResourceBucketNameHandler(test_utils.GenericTestBase):
"""Test that GCS resouce bucket name handler returns correct data."""
USERNAME_A = 'a'
EMAIL_A = 'a@example.com'

def setUp(self):
super(GcsResourceBucketNameHandler, self).setUp()
self.expected_application_id = test_utils.TestBase.EXPECTED_TEST_APP_ID
self.expected_bucket_name = (
'%s-resources' % self.expected_application_id)

def test_gcs_resource_bucket_name_handler(self):
"""Test returns correct app identity."""

self.signup(self.EMAIL_A, self.USERNAME_A)
self.login(self.EMAIL_A)

with self.swap(constants, 'DEV_MODE', False):
json_response = self.get_json(
feconf.GCS_RESOURCE_BUCKET_NAME_HANDLER_URL)
self.assertDictEqual(
{'GCS_RESOURCE_BUCKET_NAME': self.expected_bucket_name},
json_response
)

with self.swap(constants, 'DEV_MODE', True):
json_response = self.get_json(
feconf.GCS_RESOURCE_BUCKET_NAME_HANDLER_URL)
self.assertDictEqual(
{'GCS_RESOURCE_BUCKET_NAME': None},
json_response
)
self.logout()
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/base.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
ADDITIONAL_ANGULAR_MODULES: JSON.parse(
'{{additional_angular_modules|js_string}}'),
status_code: JSON.parse('{{status_code}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
};
</script>

Expand Down
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/creator-dashboard-page/creator-dashboard-page.mainpage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
var GLOBALS = {
ADDITIONAL_ANGULAR_MODULES: JSON.parse(
'{{additional_angular_modules|js_string}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
INTERACTION_SPECS: JSON.parse('{{INTERACTION_SPECS|js_string}}')
};
</script>
Expand Down
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/exploration-player-page/exploration-player-page.mainpage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
'{{additional_angular_modules|js_string}}'),
collectionId: JSON.parse('{{collection_id|js_string}}'),
collectionTitle: JSON.parse('{{collection_title|js_string}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
explorationVersion: JSON.parse(
'{{exploration_version|js_string}}'),
INTERACTION_SPECS: JSON.parse(
Expand Down
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/practice-session-page/practice-session-page.mainpage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
@require('../../base_components/header.html', {"title": "Oppia"})
<script>
var GLOBALS = {
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
INTERACTION_SPECS: JSON.parse('{{INTERACTION_SPECS|js_string}}')
}
</script>
Expand Down
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/review-test-page/review-test-page.mainpage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
var GLOBALS = {
ADDITIONAL_ANGULAR_MODULES: JSON.parse(
'{{additional_angular_modules|js_string}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
INTERACTION_SPECS: JSON.parse('{{INTERACTION_SPECS|js_string}}')
}
</script>
Expand Down
1 change: 0 additions & 1 deletion core/templates/dev/head/pages/skill-editor-page/skill-editor-page.mainpage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
var GLOBALS = {
ADDITIONAL_ANGULAR_MODULES: JSON.parse(
'{{additional_angular_modules|js_string}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
INTERACTION_SPECS: JSON.parse('{{INTERACTION_SPECS|js_string}}')
};
</script>
Expand Down
2 changes: 0 additions & 2 deletions core/templates/dev/head/pages/topic-editor-page/topic-editor-page.mainpage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
var GLOBALS = {
ADDITIONAL_ANGULAR_MODULES: JSON.parse(
'{{additional_angular_modules|js_string}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse(
'{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
INTERACTION_SPECS: JSON.parse('{{INTERACTION_SPECS|js_string}}'),
};
</script>
Expand Down
189 changes: 108 additions & 81 deletions core/templates/dev/head/services/AssetsBackendApiService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,50 @@ oppia.factory('AssetsBackendApiService', [
$http, $q, AudioFileObjectFactory, CsrfTokenService,
FileDownloadRequestObjectFactory, ImageFileObjectFactory,
UrlInterpolationService, DEV_MODE) {
if (!DEV_MODE && !GLOBALS.GCS_RESOURCE_BUCKET_NAME) {
throw Error('GCS_RESOURCE_BUCKET_NAME is not set in prod.');
}
var _initialized = false;
var GCS_RESOURCE_BUCKET_NAME = null;
var ASSET_TYPE_AUDIO = 'audio';
var ASSET_TYPE_IMAGE = 'image';
var GCS_PREFIX = null;
var AUDIO_DOWNLOAD_URL_TEMPLATE = null;
var IMAGE_DOWNLOAD_URL_TEMPLATE = null;
var AUDIO_UPLOAD_URL_TEMPLATE =
'/createhandler/audioupload/<exploration_id>';

// List of filenames that have been requested for but have
// yet to return a response.
var _audioFilesCurrentlyBeingRequested = [];
var _imageFilesCurrentlyBeingRequested = [];

var ASSET_TYPE_AUDIO = 'audio';
var ASSET_TYPE_IMAGE = 'image';
var _initialize = function() {
if (_initialized) {
return $q.resolve();
}

var GCS_PREFIX = ('https://storage.googleapis.com/' +
GLOBALS.GCS_RESOURCE_BUCKET_NAME + '/exploration');
var AUDIO_DOWNLOAD_URL_TEMPLATE = (
(DEV_MODE ? '/assetsdevhandler' : GCS_PREFIX) +
'/<exploration_id>/assets/audio/<filename>');
var IMAGE_DOWNLOAD_URL_TEMPLATE = (
(DEV_MODE ? '/assetsdevhandler' : GCS_PREFIX) +
'/<exploration_id>/assets/image/<filename>');
if (DEV_MODE) {
AUDIO_DOWNLOAD_URL_TEMPLATE = (
'/assetsdevhandler/<exploration_id>/assets/audio/<filename>');
IMAGE_DOWNLOAD_URL_TEMPLATE = (
'/assetsdevhandler/<exploration_id>/assets/image/<filename>');
_initialized = true;
return $q.resolve();
}

var AUDIO_UPLOAD_URL_TEMPLATE =
'/createhandler/audioupload/<exploration_id>';
return $http.get('/gcs_resource_bucket_name_handler')
.then(function(response) {
if (!response.data.GCS_RESOURCE_BUCKET_NAME) {
throw Error('GCS_RESOURCE_BUCKET_NAME is not set in prod.');
}
GCS_RESOURCE_BUCKET_NAME = response.data.GCS_RESOURCE_BUCKET_NAME;
GCS_PREFIX = ('https://storage.googleapis.com/' +
GCS_RESOURCE_BUCKET_NAME + '/exploration');
AUDIO_DOWNLOAD_URL_TEMPLATE = (
GCS_PREFIX + '/<exploration_id>/assets/audio/<filename>');
IMAGE_DOWNLOAD_URL_TEMPLATE = (
GCS_PREFIX + '/<exploration_id>/assets/image/<filename>');
_initialized = true;
});
};

// Map from asset filename to asset blob.
var assetsCache = {};
Expand All @@ -70,63 +91,66 @@ oppia.factory('AssetsBackendApiService', [
FileDownloadRequestObjectFactory.createNew(filename, canceler));
}

$http({
method: 'GET',
responseType: 'blob',
url: _getDownloadUrl(explorationId, filename, assetType),
timeout: canceler.promise
}).success(function(data) {
var assetBlob = null;
try {
if (assetType === ASSET_TYPE_AUDIO) {
// Add type for audio assets. Without this, translations can
// not be played on Safari.
assetBlob = new Blob([data], {type: 'audio/mpeg'});
} else {
assetBlob = new Blob([data]);
}
} catch (exception) {
window.BlobBuilder = window.BlobBuilder ||
window.WebKitBlobBuilder ||
window.MozBlobBuilder ||
window.MSBlobBuilder;
if (exception.name === 'TypeError' && window.BlobBuilder) {
_getDownloadUrlAsync(explorationId, filename, assetType)
.then(function(url) {
$http({
method: 'GET',
responseType: 'blob',
url: url,
timeout: canceler.promise
}).success(function(data) {
var assetBlob = null;
try {
var blobBuilder = new BlobBuilder();
blobBuilder.append(data);
assetBlob = blobBuilder.getBlob(assetType.concat('/*'));
} catch (e) {
var additionalInfo = (
'\nBlobBuilder construction error debug logs:' +
'\nAsset type: ' + assetType +
'\nData: ' + data
);
e.message += additionalInfo;
throw e;
if (assetType === ASSET_TYPE_AUDIO) {
// Add type for audio assets. Without this, translations can
// not be played on Safari.
assetBlob = new Blob([data], {type: 'audio/mpeg'});
} else {
assetBlob = new Blob([data]);
}
} catch (exception) {
window.BlobBuilder = window.BlobBuilder ||
window.WebKitBlobBuilder ||
window.MozBlobBuilder ||
window.MSBlobBuilder;
if (exception.name === 'TypeError' && window.BlobBuilder) {
try {
var blobBuilder = new BlobBuilder();
blobBuilder.append(data);
assetBlob = blobBuilder.getBlob(assetType.concat('/*'));
} catch (e) {
var additionalInfo = (
'\nBlobBuilder construction error debug logs:' +
'\nAsset type: ' + assetType +
'\nData: ' + data
);
e.message += additionalInfo;
throw e;
}
} else {
var additionalInfo = (
'\nBlob construction error debug logs:' +
'\nAsset type: ' + assetType +
'\nData: ' + data
);
exception.message += additionalInfo;
throw exception;
}
}
} else {
var additionalInfo = (
'\nBlob construction error debug logs:' +
'\nAsset type: ' + assetType +
'\nData: ' + data
);
exception.message += additionalInfo;
throw exception;
}
}
assetsCache[filename] = assetBlob;
if (assetType === ASSET_TYPE_AUDIO) {
successCallback(
AudioFileObjectFactory.createNew(filename, assetBlob));
} else {
successCallback(
ImageFileObjectFactory.createNew(filename, assetBlob));
}
}).error(function() {
errorCallback(filename);
})['finally'](function() {
_removeFromFilesCurrentlyBeingRequested(filename, assetType);
});
assetsCache[filename] = assetBlob;
if (assetType === ASSET_TYPE_AUDIO) {
successCallback(
AudioFileObjectFactory.createNew(filename, assetBlob));
} else {
successCallback(
ImageFileObjectFactory.createNew(filename, assetBlob));
}
}).error(function() {
errorCallback(filename);
})['finally'](function() {
_removeFromFilesCurrentlyBeingRequested(filename, assetType);
});
});
};

var _abortAllCurrentDownloads = function(assetType) {
Expand Down Expand Up @@ -203,13 +227,16 @@ oppia.factory('AssetsBackendApiService', [
});
};

var _getDownloadUrl = function(explorationId, filename, assetType) {
return UrlInterpolationService.interpolateUrl(
(assetType === ASSET_TYPE_AUDIO ? AUDIO_DOWNLOAD_URL_TEMPLATE :
IMAGE_DOWNLOAD_URL_TEMPLATE), {
exploration_id: explorationId,
filename: filename
});
var _getDownloadUrlAsync = function(explorationId, filename, assetType) {
return _initialize().then(function() {
return UrlInterpolationService.interpolateUrl(
(assetType === ASSET_TYPE_AUDIO ? AUDIO_DOWNLOAD_URL_TEMPLATE :
IMAGE_DOWNLOAD_URL_TEMPLATE), {
exploration_id: explorationId,
filename: filename
}
);
});
};

var _getAudioUploadUrl = function(explorationId) {
Expand Down Expand Up @@ -265,8 +292,8 @@ oppia.factory('AssetsBackendApiService', [
isCached: function(filename) {
return _isCached(filename);
},
getAudioDownloadUrl: function(explorationId, filename) {
return _getDownloadUrl(explorationId, filename, ASSET_TYPE_AUDIO);
getAudioDownloadUrlAsync: function(explorationId, filename) {
return _getDownloadUrlAsync(explorationId, filename, ASSET_TYPE_AUDIO);
},
abortAllCurrentAudioDownloads: function() {
_abortAllCurrentDownloads(ASSET_TYPE_AUDIO);
Expand All @@ -279,8 +306,8 @@ oppia.factory('AssetsBackendApiService', [
image: _imageFilesCurrentlyBeingRequested
};
},
getImageUrlForPreview: function(explorationId, filename) {
return _getDownloadUrl(explorationId, filename, ASSET_TYPE_IMAGE);
getImageUrlForPreviewAsync: function(explorationId, filename) {
return _getDownloadUrlAsync(explorationId, filename, ASSET_TYPE_IMAGE);
}
};
}
Expand Down
7 changes: 4 additions & 3 deletions core/templates/dev/head/services/AssetsBackendApiServiceSpec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ describe('Assets Backend API Service', function() {
it('should correctly formulate the download URL', function() {
// TODO(sll): Find a way to substitute out constants.DEV_MODE so that we
// can test the production URL, too.
expect(
AssetsBackendApiService.getAudioDownloadUrl('expid12345', 'a.mp3')
).toEqual('/assetsdevhandler/expid12345/assets/audio/a.mp3');
AssetsBackendApiService.getAudioDownloadUrlAsync('expid12345', 'a.mp3')
.then(function(url) {
expect(url).toEqual('/assetsdevhandler/expid12345/assets/audio/a.mp3');
});
});

it('should successfully fetch and cache audio', function() {
Expand Down
1 change: 0 additions & 1 deletion core/tests/build_sources/templates/base.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
'{{additional_angular_modules|js_string}}'),
DEV_MODE: JSON.parse('{{DEV_MODE|js_string}}'),
status_code: JSON.parse('{{status_code}}'),
GCS_RESOURCE_BUCKET_NAME: JSON.parse('{{GCS_RESOURCE_BUCKET_NAME|js_string}}'),
isAdmin: JSON.parse('{{is_admin|js_string}}'),
isModerator: JSON.parse('{{is_moderator|js_string}}'),
isSuperAdmin: JSON.parse('{{is_super_admin|js_string}}'),
Expand Down
Loading

0 comments on commit a7bb506

Please sign in to comment.