Skip to content

Commit

Permalink
Fix oppia#2447: Caching should be done on a per-file basis (oppia#3454)
Browse files Browse the repository at this point in the history
* individual file cache slugs

* cache slug system changed

* interpolationg of images, directives, translates

* Subject images proper load

* use Jinja for css url interpolation and use athor function for loading overlays

* prod loading

* fix tests

* linting fixes

* removed print

* renamed constatant

* part of comments addressed

* changed extension loading, adressed comments

* next batch of comments adressed

* fix tests

* UrlInterpolation tests

* build tests and test fixes

* fix linting errors

* addressed comment

* fix tests and remove cssBackgroundImages
  • Loading branch information
vojtechjelinek authored Jul 22, 2017
1 parent 3847d75 commit 41291a0
Show file tree
Hide file tree
Showing 47 changed files with 696 additions and 270 deletions.
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
core/templates/dev/head/expressions/parser.js
core/templates/prod/*
backend_prod_files/*
core/tests/protractor.conf.js
extensions/interactions/LogicProof/static/js/generatedDefaultData.js
extensions/interactions/LogicProof/static/js/generatedParser.js
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*.swo
gae_runtime/*
third_party/*
core/templates/prod/*
backend_prod_files/*
.coverage*
.viminfo
libpeerconnection.log
Expand Down
14 changes: 5 additions & 9 deletions app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,12 @@ handlers:
application_readable: true
http_headers:
Cache-Control: 'public, max-age=60'
- url: /css
# NB: not minified. TODO(sll): fix.
static_dir: core/templates/dev/head/css
secure: always
# Serve js scripts under core/templates/dev/head. This regex allows
# us to recursively serve js scripts. "\1" inserts text captured by
# the capture group in the URL pattern.
- url: /templates/dev/head/(.*\.(js))$
# Serve js scripts and css files under core/templates/dev/head.
# This regex allows us to recursively serve js scripts.
# "\1" inserts text captured by the capture group in the URL pattern.
- url: /templates/dev/head/(.*\.(js|css))$
static_files: core/templates/dev/head/\1
upload: core/templates/dev/head/(.*\.(js))$
upload: core/templates/dev/head/(.*\.(js|css))$
secure: always
- url: /templates/dev/head/(.*\.(html))$
static_files: core/templates/dev/head/\1
Expand Down
5 changes: 0 additions & 5 deletions cache_slug.yaml

This file was deleted.

24 changes: 8 additions & 16 deletions core/domain/summary_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,7 @@ def test_get_displayable_exp_summary_dicts_matching_ids(self):
'status': 'public',
'tags': [],
'thumbnail_bg_color': '#a33f40',
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'title': u'Exploration 2 Albert title',
}
self.assertIn('last_updated_msec', displayable_summaries[0])
Expand Down Expand Up @@ -273,8 +272,7 @@ def test_get_library_groups(self):
'tags': [],
'title': u'The Lazy Magician',
'thumbnail_bg_color': '#d0982a',
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Algorithms.svg'),
'thumbnail_icon_url': '/subjects/Algorithms.svg',
}
expected_group = {
'categories': ['Algorithms', 'Computing', 'Programming'],
Expand Down Expand Up @@ -350,8 +348,7 @@ def test_for_featured_explorations(self):
'thumbnail_bg_color': '#a33f40',
'community_owned': False,
'tags': [],
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'language_code': constants.DEFAULT_LANGUAGE_CODE,
'id': self.EXP_ID_2,
'category': 'A category',
Expand Down Expand Up @@ -638,9 +635,8 @@ def test_at_most_eight_top_rated_explorations(self):
'thumbnail_bg_color': '#a33f40',
'community_owned': False,
'tags': [],
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'language_code': constants.DEFAULT_LANGUAGE_CODE,
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'id': self.EXP_ID_3,
'category': u'A category',
'ratings': {u'1': 0, u'3': 0, u'2': 0, u'5': 1, u'4': 1},
Expand Down Expand Up @@ -677,8 +673,7 @@ def test_only_explorations_with_ratings_are_returned(self):
'thumbnail_bg_color': '#a33f40',
'community_owned': False,
'tags': [],
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'language_code': constants.DEFAULT_LANGUAGE_CODE,
'id': self.EXP_ID_2,
'category': u'A category',
Expand Down Expand Up @@ -760,8 +755,7 @@ def test_for_recently_published_explorations(self):
'thumbnail_bg_color': '#a33f40',
'community_owned': False,
'tags': [],
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'language_code': constants.DEFAULT_LANGUAGE_CODE,
'id': self.EXP_ID_1,
'category': u'A category',
Expand All @@ -775,8 +769,7 @@ def test_for_recently_published_explorations(self):
'thumbnail_bg_color': '#a33f40',
'community_owned': False,
'tags': [],
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'language_code': constants.DEFAULT_LANGUAGE_CODE,
'id': self.EXP_ID_2,
'category': u'A category',
Expand All @@ -790,8 +783,7 @@ def test_for_recently_published_explorations(self):
'thumbnail_bg_color': '#a33f40',
'community_owned': False,
'tags': [],
'thumbnail_icon_url': self.get_static_asset_url(
'/images/subjects/Lightbulb.svg'),
'thumbnail_icon_url': '/subjects/Lightbulb.svg',
'language_code': constants.DEFAULT_LANGUAGE_CODE,
'id': self.EXP_ID_3,
'category': u'A category',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ oppia.directive('collectionSummaryTile', [
}
);
};

$scope.getCompleteThumbnailIconUrl = function () {
return UrlInterpolationService.getStaticImageUrl(
$scope.getThumbnailIconUrl());
};

$scope.getStaticImageUrl = function (url) {
return UrlInterpolationService.getStaticImageUrl(url);
};
}
]
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ oppia.directive('explorationSummaryTile', [
windowDimensionsService.getWidth() >= $scope.mobileCutoffPx);
$scope.$apply();
});

$scope.getCompleteThumbnailIconUrl = function () {
return UrlInterpolationService.getStaticImageUrl(
$scope.getThumbnailIconUrl());
};
}
]
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<md-card class="oppia-activity-summary-tile">
<a ng-href="<[getCollectionLink()]>" style="text-decoration: none;">
<div class="title-section" style="background-color: <[getThumbnailBgColor()]>;">
<img class="collection-corner-image" ng-src="/assets/images/general/collection_corner.svg" alt="">
<img class="thumbnail-image" ng-src="<[getThumbnailIconUrl()]>" alt="<[getCategory()]>">
<img class="collection-corner-image" ng-src="<[getStaticImageUrl('/general/collection_corner.svg')]>" alt="">
<img class="thumbnail-image" ng-src="<[getCompleteThumbnailIconUrl()]>" alt="<[getCategory()]>">
<h3 class="activity-title protractor-test-collection-summary-tile-title"><[getCollectionTitle() || DEFAULT_EMPTY_TITLE]></h3>
</div>

<div>
<div>
<div class="objective">
<[getObjective() | truncateAndCapitalize: 95]>
<span ng-if="!getObjective()">No objective specified.</span>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<md-card ng-class="[isCollectionPreviewTile ? 'oppia-activity-summary-tile-mobile' : 'oppia-activity-summary-tile', {'small-width': !isWindowLarge}]">
<a ng-href="<[getExplorationLink()]>" target="<[openInNewWindow ? '_blank' : '_self']>">
<div class="title-section" style="background-color: <[getThumbnailBgColor()]>;">
<img class="thumbnail-image" ng-src="<[getThumbnailIconUrl()]>" alt="<[getCategory()]>">
<img class="thumbnail-image" ng-src="<[getCompleteThumbnailIconUrl()]>" alt="<[getCategory()]>">
<h3 class="activity-title protractor-test-exp-summary-tile-title">
<span ng-if="isWindowLarge"><[getExplorationTitle()|truncate:40]></span>
<span ng-if="!isWindowLarge"><[getExplorationTitle()|truncate:40]></span>
Expand All @@ -20,7 +20,7 @@ <h3 class="activity-title protractor-test-exp-summary-tile-title">
</div>
</div>
<div class="title-section-mask"></div>
<div ng-attr-section="<[isWindowLarge ? undefined : 'right-section']>">
<div ng-attr-section="<[isWindowLarge ? undefined : 'right-section']>">
<div ng-if="isWindowLarge" class="objective protractor-test-exp-summary-tile-objective">
<[getObjective() | truncateAndCapitalize: 95]>
</div>
Expand Down
79 changes: 56 additions & 23 deletions core/templates/dev/head/domain/utilities/UrlInterpolationService.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,37 @@ oppia.factory('UrlInterpolationService', [
}
};

var getCachePrefixedUrl = function(resourcePath) {
/**
* Given a resource path relative to subfolder in /,
* returns resource path with cache slug.
*/
var getUrlWithSlug = function(resourcePath) {
if (GLOBALS.MINIFICATION || !GLOBALS.DEV_MODE) {
if (hashes[resourcePath]) {
var index = resourcePath.lastIndexOf('.');
return (resourcePath.slice(0, index) + '.' + hashes[resourcePath] +
resourcePath.slice(index));
}
}
return resourcePath;
};

/**
* Given a resource path relative to subfolder in /,
* returns complete resource path with cache slug and prefixed with url
* depending on dev/prod mode.
*/
var getCompleteUrl = function(prefix, path) {
return GLOBALS.ASSET_DIR_PREFIX + prefix + getUrlWithSlug(path);
};

/**
* Given a resource path relative to extensions folder,
* returns the complete url path to that resource.
*/
var getExtensionResourceUrl = function(resourcePath) {
validateResourcePath(resourcePath);
return GLOBALS.ASSET_DIR_PREFIX + resourcePath;
return getCompleteUrl('/extensions', resourcePath);
};

return {
Expand Down Expand Up @@ -130,57 +158,62 @@ oppia.factory('UrlInterpolationService', [
},

/**
* Given an resource path, returns the relative url path to that resource
* prefixing the appropriate cache_slug to it.
*/
getStaticResourceUrl: function(resourcePath) {
return getCachePrefixedUrl(resourcePath);
},

/**
* Given an image path, returns the complete url path to that image
* prefixing the appropriate cache_slug to it.
* Given an image path relative to /assets/images folder,
* returns the complete url path to that image.
*/
getStaticImageUrl: function(imagePath) {
validateResourcePath(imagePath);
return getCachePrefixedUrl('/assets/images' + imagePath);
return getCompleteUrl('/assets', '/images' + imagePath);
},

/**
* Given a gadget type, returns the complete url path to that
* gadget type image, prefixing the appropriate cache_slug to it.
* gadget type image.
*/
getGadgetImgUrl: function(gadgetType) {
if (!gadgetType) {
alertsService.fatalWarning(
'Empty gadgetType passed in getGadgetImgUrl.');
}
return getCachePrefixedUrl('/extensions/gadgets/' + gadgetType +
return getExtensionResourceUrl('/gadgets/' + gadgetType +
'/static/images/' + gadgetType + '.png');
},

/**
* Given an interaction id, returns the complete url path to the thumbnail
* image for the interaction, prefixing the appropriate cache_slug to it.
* Given an interaction id, returns the complete url path to
* the thumbnail image for the interaction.
*/
getInteractionThumbnailImageUrl: function(interactionId) {
if (!interactionId) {
alertsService.fatalWarning(
'Empty interactionId passed in getInteractionThumbnailImageUrl.');
}
return getCachePrefixedUrl('/extensions/interactions/' +
interactionId + '/static/' + interactionId + '.png');
return getExtensionResourceUrl('/interactions/' + interactionId +
'/static/' + interactionId + '.png');
},

/**
* Given a directive path relative to head folder,
* returns the complete url path to that directive, prefixing the
* appropriate cache_slug to it.
* returns the complete url path to that directive.
*/
getDirectiveTemplateUrl: function(path) {
validateResourcePath(path);
return GLOBALS.TEMPLATE_DIR_PREFIX + path;
}
return GLOBALS.TEMPLATE_DIR_PREFIX + getUrlWithSlug(path);
},

/**
* Given a json path relative to assets folder,
* returns the complete url path to that json.
*/
getTranslateJsonUrl: function(jsonPath) {
validateResourcePath(jsonPath);
return getCompleteUrl('/assets', jsonPath);
},

getExtensionResourceUrl: getExtensionResourceUrl,

_getUrlWithSlug: getUrlWithSlug,
_getCompleteUrl: getCompleteUrl
};
}
]);
Loading

0 comments on commit 41291a0

Please sign in to comment.