Skip to content

Commit

Permalink
Revert "Fix part of oppia#5002 remove GCS_RESOURCE_BUCKET_NAME from G…
Browse files Browse the repository at this point in the history 
…LOBALS (oppia#7130)" (oppia#7278)

This reverts commit a7bb506.
  • Loading branch information
@jameesjohn @seanlip
jameesjohn authored and seanlip committed Aug 3, 2019
1 parent 6b7cb9d commit 2570a9e
Show file tree
Hide file tree
Showing 19 changed files with 124 additions and 213 deletions.
3 changes: 3 additions & 0 deletions core/controllers/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
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 @@ -304,6 +305,8 @@ 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: 0 additions & 16 deletions core/controllers/resources.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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 @@ -105,18 +104,3 @@ 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: 0 additions & 35 deletions core/controllers/resources_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,38 +606,3 @@ 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: 1 addition & 0 deletions core/templates/dev/head/pages/base.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
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: 1 addition & 0 deletions 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,6 +6,7 @@
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: 1 addition & 0 deletions 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,6 +9,7 @@
'{{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: 1 addition & 0 deletions 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,6 +4,7 @@
@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: 1 addition & 0 deletions 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,6 +7,7 @@
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: 1 addition & 0 deletions 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,6 +6,7 @@
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: 2 additions & 0 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,6 +6,8 @@
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: 81 additions & 108 deletions core/templates/dev/head/services/AssetsBackendApiService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,50 +31,29 @@ angular.module('oppia').factory('AssetsBackendApiService', [
$http, $q, AudioFileObjectFactory, CsrfTokenService,
FileDownloadRequestObjectFactory, ImageFileObjectFactory,
UrlInterpolationService, DEV_MODE) {
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>';
if (!DEV_MODE && !GLOBALS.GCS_RESOURCE_BUCKET_NAME) {
throw Error('GCS_RESOURCE_BUCKET_NAME is not set in prod.');
}

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

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

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 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>');

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;
});
};
var AUDIO_UPLOAD_URL_TEMPLATE =
'/createhandler/audioupload/<exploration_id>';

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

_getDownloadUrlAsync(explorationId, filename, assetType)
.then(function(url) {
$http({
method: 'GET',
responseType: 'blob',
url: url,
timeout: canceler.promise
}).success(function(data) {
var assetBlob = null;
$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) {
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) {
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;
}
}
assetsCache[filename] = assetBlob;
if (assetType === ASSET_TYPE_AUDIO) {
successCallback(
AudioFileObjectFactory.createNew(filename, assetBlob));
} else {
successCallback(
ImageFileObjectFactory.createNew(filename, assetBlob));
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;
}
}).error(function() {
errorCallback(filename);
})['finally'](function() {
_removeFromFilesCurrentlyBeingRequested(filename, assetType);
});
});
} 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);
});
};

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

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 _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 _getAudioUploadUrl = function(explorationId) {
Expand Down Expand Up @@ -290,8 +263,8 @@ angular.module('oppia').factory('AssetsBackendApiService', [
isCached: function(filename) {
return _isCached(filename);
},
getAudioDownloadUrlAsync: function(explorationId, filename) {
return _getDownloadUrlAsync(explorationId, filename, ASSET_TYPE_AUDIO);
getAudioDownloadUrl: function(explorationId, filename) {
return _getDownloadUrl(explorationId, filename, ASSET_TYPE_AUDIO);
},
abortAllCurrentAudioDownloads: function() {
_abortAllCurrentDownloads(ASSET_TYPE_AUDIO);
Expand All @@ -304,8 +277,8 @@ angular.module('oppia').factory('AssetsBackendApiService', [
image: _imageFilesCurrentlyBeingRequested
};
},
getImageUrlForPreviewAsync: function(explorationId, filename) {
return _getDownloadUrlAsync(explorationId, filename, ASSET_TYPE_IMAGE);
getImageUrlForPreview: function(explorationId, filename) {
return _getDownloadUrl(explorationId, filename, ASSET_TYPE_IMAGE);
}
};
}
Expand Down
7 changes: 3 additions & 4 deletions core/templates/dev/head/services/AssetsBackendApiServiceSpec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ 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.
AssetsBackendApiService.getAudioDownloadUrlAsync('expid12345', 'a.mp3')
.then(function(url) {
expect(url).toEqual('/assetsdevhandler/expid12345/assets/audio/a.mp3');
});
expect(
AssetsBackendApiService.getAudioDownloadUrl('expid12345', 'a.mp3')
).toEqual('/assetsdevhandler/expid12345/assets/audio/a.mp3');
});

it('should successfully fetch and cache audio', function() {
Expand Down
1 change: 1 addition & 0 deletions core/tests/build_sources/templates/base.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
'{{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 2570a9e

Please sign in to comment.