From 19a1ca64f6b457c838b859afb91e354addb7a8d5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 1 Nov 2013 11:35:48 +1100 Subject: [PATCH] Fixes tree cache - fixes the unit tests associated with the tree cache so they run properly, tree cache can now be cleared by section, by key, by both or all. There's no longer a default cache key since we don't want to cache things when not needed. ONLY the main trees are now cached, tree pickers and dialog trees should never be cached since tree structures change all of the time (i.e. by other users, changing node names, etc...). The tree-picker directive also had a cache which meant that anyone using that directive would end up with the same cache. --- .../common/directives/umbtree.directive.js | 4 +- .../src/common/services/navigation.service.js | 2 +- .../src/common/services/tree.service.js | 79 ++++--- .../views/common/dialogs/contentpicker.html | 1 - .../src/views/common/dialogs/linkpicker.html | 1 - .../common/dialogs/membergrouppicker.html | 1 - .../views/common/dialogs/memberpicker.html | 1 - .../src/views/common/dialogs/treepicker.html | 1 - .../src/views/content/copy.html | 2 +- .../src/views/content/move.html | 2 +- .../src/views/directives/umb-navigation.html | 1 + .../src/views/media/move.html | 1 - .../unit/common/services/tree-service.spec.js | 201 ++++++++++++++---- 13 files changed, 218 insertions(+), 79 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/umbtree.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/umbtree.directive.js index 15fa9ef5dc..00fa4e57e7 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/umbtree.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/umbtree.directive.js @@ -70,7 +70,7 @@ angular.module("umbraco.directives") if (scope.eventhandler) { scope.eventhandler.clearCache = function(section){ - treeService.clearCache(section); + treeService.clearCache({ section: section }); }; scope.eventhandler.load = function(section){ @@ -110,7 +110,7 @@ angular.module("umbraco.directives") enableDeleteAnimations = false; //use $q.when because a promise OR raw data might be returned. - $q.when(treeService.getTree({ section: scope.section, tree: scope.treealias, cachekey: scope.cachekey, isDialog: scope.isdialog ? scope.isdialog : false })) + treeService.getTree({ section: scope.section, tree: scope.treealias, cacheKey: scope.cachekey, isDialog: scope.isdialog ? scope.isdialog : false }) .then(function (data) { //set the data once we have it scope.tree = data; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js index f8b0c8fef6..adef8a100c 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js @@ -288,7 +288,7 @@ angular.module('umbraco.services') reloadSection: function (sectionAlias) { if(this.ui.treeEventHandler){ - this.ui.treeEventHandler.clearCache(sectionAlias); + this.ui.treeEventHandler.clearCache({ section: sectionAlias }); this.ui.treeEventHandler.load(sectionAlias); } }, diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js index 8d294126bf..7288c570ec 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js @@ -15,14 +15,12 @@ function treeService($q, treeResource, iconHelper, notificationsService, $rootSc var standardCssClass = 'icon umb-tree-icon sprTree'; function getCacheKey(args) { - if (!args) { - args = { - section: 'content', - cacheKey: '' - }; - } + //if there is no cache key they return null - it won't be cached. + if (!args || !args.cacheKey) { + return null; + } - var cacheKey = args.cachekey; + var cacheKey = args.cacheKey; cacheKey += "_" + args.section; return cacheKey; } @@ -103,15 +101,35 @@ function treeService($q, treeResource, iconHelper, notificationsService, $rootSc return undefined; }, - /** clears the tree cache - with optional sectionAlias */ - clearCache: function(sectionAlias) { - if (!sectionAlias) { + /** clears the tree cache - with optional cacheKey and optional section */ + clearCache: function (args) { + //clear all if not specified + if (!args) { treeCache = {}; } else { - var cacheKey = getCacheKey({ section: sectionAlias }); - if (treeCache && treeCache[cacheKey] != null) { - treeCache = _.omit(treeCache, cacheKey); + //if section and cache key specified just clear that cache + if (args.section && args.cacheKey) { + var cacheKey = getCacheKey(args); + if (cacheKey && treeCache && treeCache[cacheKey] != null) { + treeCache = _.omit(treeCache, cacheKey); + } + } + else if (args.cacheKey) { + //if only the cache key is specified, then clear all cache starting with that key + var allKeys1 = _.keys(treeCache); + var toRemove1 = _.filter(allKeys1, function (k) { + return k.startsWith(args.cacheKey + "_"); + }); + treeCache = _.omit(treeCache, toRemove1); + } + else if (args.section) { + //if only the section is specified then clear all cache regardless of cache key by that section + var allKeys2 = _.keys(treeCache); + var toRemove2 = _.filter(allKeys2, function (k) { + return k.endsWith("_" + args.section); + }); + treeCache = _.omit(treeCache, toRemove2); } } }, @@ -240,25 +258,28 @@ function treeService($q, treeResource, iconHelper, notificationsService, $rootSc return ""; }, + /** gets the tree, returns a promise */ getTree: function (args) { + var deferred = $q.defer(); + //set defaults if (!args) { - args = { - section: 'content', - cacheKey : '' - }; + args = { section: 'content', cacheKey: null }; + } + else if (!args.section) { + args.section = 'content'; } var cacheKey = getCacheKey(args); //return the cache if it exists - if (treeCache[cacheKey] !== undefined){ - return treeCache[cacheKey]; + if (cacheKey && treeCache[cacheKey] !== undefined) { + deferred.resolve(treeCache[cacheKey]); } var self = this; - return treeResource.loadApplication(args) + treeResource.loadApplication(args) .then(function(data) { //this will be called once the tree app data has loaded var result = { @@ -268,13 +289,19 @@ function treeService($q, treeResource, iconHelper, notificationsService, $rootSc }; //we need to format/modify some of the node data to be used in our app. self._formatNodeDataForUseInUI(result.root, result.root.children, args.section); - //cache this result - //TODO: We'll need to un-cache this in many circumstances - treeCache[cacheKey] = result; - //return the data result as promised - //deferred.resolve(treeArray[cacheKey]); - return treeCache[cacheKey]; + + //cache this result if a cache key is specified - generally a cache key should ONLY + // be specified for application trees, dialog trees should not be cached. + if (cacheKey) { + treeCache[cacheKey] = result; + deferred.resolve(treeCache[cacheKey]); + } + + //return un-cached + deferred.resolve(result); }); + + return deferred.promise; }, getMenu: function (args) { diff --git a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/contentpicker.html b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/contentpicker.html index 3da0a7e320..7890421360 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/contentpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/contentpicker.html @@ -38,7 +38,6 @@
diff --git a/src/Umbraco.Web.UI.Client/src/views/content/move.html b/src/Umbraco.Web.UI.Client/src/views/content/move.html index 2a288bcde8..5456118374 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/move.html +++ b/src/Umbraco.Web.UI.Client/src/views/content/move.html @@ -21,9 +21,9 @@
diff --git a/src/Umbraco.Web.UI.Client/src/views/directives/umb-navigation.html b/src/Umbraco.Web.UI.Client/src/views/directives/umb-navigation.html index 714ef5b4dc..54baf2ba95 100644 --- a/src/Umbraco.Web.UI.Client/src/views/directives/umb-navigation.html +++ b/src/Umbraco.Web.UI.Client/src/views/directives/umb-navigation.html @@ -63,6 +63,7 @@
diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js index a4757a8ae4..cd79092385 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js @@ -1,5 +1,5 @@ describe('tree service tests', function () { - var treeService; + var treeService, $rootScope, $httpBackend, mocks; function getContentTree() { @@ -37,66 +37,183 @@ describe('tree service tests', function () { } beforeEach(module('umbraco.services')); + beforeEach(module('umbraco.resources')); + beforeEach(module('umbraco.mocks')); - beforeEach(inject(function ($injector) { + beforeEach(inject(function ($injector, mocksUtils) { + + //for these tests we don't want any authorization to occur + mocksUtils.disableAuth(); + + $rootScope = $injector.get('$rootScope'); + $httpBackend = $injector.get('$httpBackend'); + mocks = $injector.get("treeMocks"); + mocks.register(); treeService = $injector.get('treeService'); })); describe('tree cache', function () { - it('tree with no args caches as content', function () { - treeService.getTree().then(function(data) { + //it('tree with section but no cache key does not cache', function () { + // treeService.getTree().then(function (data) { - var cache = treeService._getTreeCache(); - expect(cache).toBeDefined(); - expect(cache["_content"]).toBeDefined(); + // var cache = treeService._getTreeCache(); + // expect(cache).toBeDefined(); + // expect(cache["_content"]).toBeDefined(); - }); - }); - - it('tree caches by section', function () { - treeService.getTree({section: "media"}).then(function (data) { + // }); + //}); - var cache = treeService._getTreeCache(); - expect(cache).toBeDefined(); - expect(cache["_media"]).toBeDefined(); + it('does not cache with no args', function () { - }); - }); - - it('removes cache by section', function () { - treeService.getTree({ section: "media" }).then(function (data) { - - var cache = treeService._getTreeCache(); - expect(cache["_media"]).toBeDefined(); - expect(_.keys(cache).length).toBe(1); - - treeService.clearCache("media"); - + var cache; + treeService.getTree().then(function (data) { cache = treeService._getTreeCache(); - expect(cache["_media"]).not.toBeDefined(); - expect(_.keys(cache).length).toBe(0); }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(_.keys(cache).length).toBe(0); + }); + + it('does not cache with no cacheKey', function () { + var cache; + treeService.getTree({section: "content"}).then(function (data) { + cache = treeService._getTreeCache(); + + }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(_.keys(cache).length).toBe(0); + }); + + it('caches by section with cache key', function () { + var cache; + treeService.getTree({ section: "media", cacheKey: "_" }).then(function (data) { + cache = treeService._getTreeCache(); + }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(cache["__media"]).toBeDefined(); + }); + + it('caches by default content section with cache key', function () { + var cache; + treeService.getTree({ cacheKey: "_" }).then(function (data) { + cache = treeService._getTreeCache(); + }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(cache).toBeDefined(); + expect(cache["__content"]).toBeDefined(); + }); + + it('removes by section with cache key', function () { + var cache; + treeService.getTree({ section: "media", cacheKey: "_" }).then(function (data) { + treeService.getTree({ section: "content", cacheKey: "_" }).then(function (d) { + cache = treeService._getTreeCache(); + }); + }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(cache["__media"]).toBeDefined(); + expect(_.keys(cache).length).toBe(2); + + treeService.clearCache({ section: "media", cacheKey: "_" }); + cache = treeService._getTreeCache(); + + expect(cache["__media"]).not.toBeDefined(); + expect(_.keys(cache).length).toBe(1); + + }); + + it('removes cache by key regardless of section', function () { + var cache; + + treeService.getTree({ section: "media", cacheKey: "_" }).then(function (data) { + treeService.getTree({ section: "content", cacheKey: "_" }).then(function (d) { + treeService.getTree({ section: "content", cacheKey: "anotherkey" }).then(function (dd) { + cache = treeService._getTreeCache(); + }); + }); + }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(cache["__media"]).toBeDefined(); + expect(cache["__content"]).toBeDefined(); + expect(_.keys(cache).length).toBe(3); + + treeService.clearCache({ cacheKey: "_" }); + + cache = treeService._getTreeCache(); + expect(cache["__media"]).not.toBeDefined(); + expect(cache["__content"]).not.toBeDefined(); + expect(_.keys(cache).length).toBe(1); + }); + + it('removes all section cache regardless of key', function () { + + var cache; + + treeService.getTree({ section: "media", cacheKey: "_" }).then(function (data) { + treeService.getTree({ section: "media", cacheKey: "anotherkey" }).then(function (d) { + treeService.getTree({ section: "content", cacheKey: "anotherkey" }).then(function (dd) { + cache = treeService._getTreeCache(); + }); + }); + }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(cache["anotherkey_media"]).toBeDefined(); + expect(cache["__media"]).toBeDefined(); + expect(_.keys(cache).length).toBe(3); + + treeService.clearCache({ section: "media" }); + + cache = treeService._getTreeCache(); + expect(cache["anotherkey_media"]).not.toBeDefined(); + expect(cache["__media"]).not.toBeDefined(); + expect(_.keys(cache).length).toBe(1); }); it('removes all cache', function () { - treeService.getTree({ section: "media" }).then(function (data) { - treeService.getTree({ section: "content" }).then(function (d) { + var cache; - var cache = treeService._getTreeCache(); - expect(cache["_content"]).toBeDefined(); - expect(cache["_media"]).toBeDefined(); - expect(_.keys(cache).length).toBe(2); - - treeService.clearCache(); - - cache = treeService._getTreeCache(); - expect(cache["_content"]).toBeDefined(); - expect(cache["_media"]).toBeDefined(); - expect(_.keys(cache).length).toBe(0); + treeService.getTree({ section: "media", cacheKey: "_" }).then(function (data) { + treeService.getTree({ section: "media", cacheKey: "anotherkey" }).then(function (d) { + treeService.getTree({ section: "content", cacheKey: "anotherkey" }).then(function(dd) { + cache = treeService._getTreeCache(); + }); }); }); + + $rootScope.$digest(); + $httpBackend.flush(); + + expect(cache["anotherkey_media"]).toBeDefined(); + expect(cache["__media"]).toBeDefined(); + expect(cache["anotherkey_content"]).toBeDefined(); + expect(_.keys(cache).length).toBe(3); + + treeService.clearCache(); + + cache = treeService._getTreeCache(); + expect(_.keys(cache).length).toBe(0); }); });