Skip to content

Commit

Permalink
pencilblue#962 - Corrects bug that prevents caching of plugin settings
Browse files Browse the repository at this point in the history
  • Loading branch information
@brianhyder
brianhyder committed Mar 15, 2016
1 parent ed0ee02 commit 4bc025b
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 88 deletions.
78 changes: 39 additions & 39 deletions include/service/entities/plugin_setting_service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = function PluginSettingServiceModule(pb) {
* @type {String}
*/
var GLOBAL_SITE = pb.SiteService.GLOBAL_SITE;

/**
* @private
* @static
Expand All @@ -28,14 +28,14 @@ module.exports = function PluginSettingServiceModule(pb) {
*/
function PluginSettingService(siteUID){
//construct settings services

/**
*
* @property caching
* @type {Object}
*/
this.caching = pb.config.plugins.caching;

/**
*
* @property site
Expand All @@ -56,11 +56,11 @@ module.exports = function PluginSettingServiceModule(pb) {
* @type {SimpleLayeredService}
*/
this.pluginSettingsService = genSettingsService({
objType: 'plugin_settings',
useMemory: this.caching.useMemory,
useCache: this.caching.useCache,
serviceName: 'PluginSettingService',
site: this.site,
objType: 'plugin_settings',
useMemory: this.caching.use_memory,
useCache: this.caching.use_cache,
serviceName: 'PluginSettingService',
site: this.site,
onlyThisSite: false
});

Expand All @@ -70,11 +70,11 @@ module.exports = function PluginSettingServiceModule(pb) {
* @type {SimpleLayeredService}
*/
this.themeSettingsService = genSettingsService({
objType: 'theme_settings',
useMemory: this.caching.useMemory,
useCache: this.caching.useCache,
serviceName: 'ThemeSettingService',
site: this.site,
objType: 'theme_settings',
useMemory: this.caching.use_memory,
useCache: this.caching.use_cache,
serviceName: 'ThemeSettingService',
site: this.site,
onlyThisSite: false
});
}
Expand Down Expand Up @@ -132,16 +132,16 @@ module.exports = function PluginSettingServiceModule(pb) {
};

/**
* Retrieves the settings for a plugin as hash of key/value pairs. This
* differs from the getSettings function because the getSettings function
* provides the settings in their raw form as an array of objects containing
* multiple properties. In most circumstances just the k/v pair is needed and
* not any additional information about the property. The function takes the
* raw settings array and transforms it into an object where the setting name
* Retrieves the settings for a plugin as hash of key/value pairs. This
* differs from the getSettings function because the getSettings function
* provides the settings in their raw form as an array of objects containing
* multiple properties. In most circumstances just the k/v pair is needed and
* not any additional information about the property. The function takes the
* raw settings array and transforms it into an object where the setting name
* is the property and the setting value is the value.
* @method getSettingsKV
* @param {String} pluginName The unique ID of the plugin who settings are to be retrieved
* @param {Function} cb A callback that takes two parameters. A error, if
* @param {Function} cb A callback that takes two parameters. A error, if
* exists, and a hash of of the plugin's settings' names/values.
*/
PluginSettingService.prototype.getSettingsKV = function(pluginName, cb) {
Expand Down Expand Up @@ -360,16 +360,16 @@ module.exports = function PluginSettingServiceModule(pb) {
};

/**
* Retrieves the theme settings for a plugin as hash of key/value pairs. This
* differs from the getThemeSettings function because the getThemeSettings function
* provides the settings in their raw form as an array of objects containing
* multiple properties. In most circumstances just the k/v pair is needed and
* not any additional information about the property. The function takes the
* raw settings array and transforms it into an object where the setting name
* Retrieves the theme settings for a plugin as hash of key/value pairs. This
* differs from the getThemeSettings function because the getThemeSettings function
* provides the settings in their raw form as an array of objects containing
* multiple properties. In most circumstances just the k/v pair is needed and
* not any additional information about the property. The function takes the
* raw settings array and transforms it into an object where the setting name
* is the property and the setting value is the value.
* @method getThemeSettingsKV
* @param {String} pluginName The unique ID of the plugin who settings are to be retrieved
* @param {Function} cb A callback that takes two parameters. A error, if
* @param {Function} cb A callback that takes two parameters. A error, if
* exists, and a hash of of the plugin's settings' names/values.
*/
PluginSettingService.prototype.getThemeSettingsKV = function(pluginName, cb) {
Expand Down Expand Up @@ -518,7 +518,7 @@ module.exports = function PluginSettingServiceModule(pb) {
* @return {SimpleLayeredService}
*/
function genSettingsService(opts) {

//add in-memory service
var services = [];

Expand Down Expand Up @@ -555,11 +555,11 @@ module.exports = function PluginSettingServiceModule(pb) {
function getAdminPluginSettingsService(self) {
if(!self.adminPluginSettingsService) {
var opts = {
objType: 'plugin_settings',
useMemory: self.caching.useMemory,
useCache: self.caching.useCache,
serviceName: 'PluginSettingService',
site: self.site,
objType: 'plugin_settings',
useMemory: self.caching.useMemory,
useCache: self.caching.useCache,
serviceName: 'PluginSettingService',
site: self.site,
onlyThisSite: true
};
self.adminPluginSettingsService = genSettingsService(opts);
Expand All @@ -577,17 +577,17 @@ module.exports = function PluginSettingServiceModule(pb) {
function getAdminThemeSettingsService(self) {
if(!self.adminThemeSettingsService) {
var opts = {
objType: 'theme_settings',
useMemory: self.caching.useMemory,
useCache: self.caching.useCache,
serviceName: 'ThemeSettingService',
site: self.site,
objType: 'theme_settings',
useMemory: self.caching.useMemory,
useCache: self.caching.useCache,
serviceName: 'ThemeSettingService',
site: self.site,
onlyThisSite: true
};
self.adminThemeSettingsService = genSettingsService(opts);
}
return self.adminThemeSettingsService;
}

return PluginSettingService;
};
74 changes: 27 additions & 47 deletions include/service/memory_entity_service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,18 @@ module.exports = function MemoryEntityServiceModule(pb) {
this.objType = options.objType;
this.keyField = options.keyField;
this.valueField = options.valueField ? options.valueField : null;
this.storage = STORAGE;
this.timers = TIMERS;
this.timeout = options.timeout || 0;
this.changeHandler = MemoryEntityService.createChangeHandler(this);
this.site = options.site || GLOBAL_SITE;
this.onlyThisSite = options.onlyThisSite ? true : false;

//register change handler
var commandInstance = pb.CommandService.getInstance();

//ensure we can get change events
// TODO fix memory leak here. Each instance creates a handler but the dispose is now gone. Need to make this generic.
commandInstance.registerForType(MemoryEntityService.getOnChangeType(this.objType), this.changeHandler);

//ensure we are cleaning up after ourselves
if (REAPER_HANDLE === null) {
MemoryEntityService.startReaper();
}

//ensure we can get change events. If we're already registered the function will return false
var c = pb.CommandService.getInstance()
.registerForType(MemoryEntityService.getOnChangeType(), MemoryEntityService.changeHandler);
}

/**
Expand Down Expand Up @@ -120,13 +114,9 @@ module.exports = function MemoryEntityServiceModule(pb) {
* @param {Function} cb Callback function
*/
MemoryEntityService.prototype.get = function(key, cb){
var value = null;
if(this.site) {
value = getSiteValue(this, key, this.site);
}
if(value == null && this.site !== GLOBAL_SITE && !this.onlyThisSite) {
value = getGlobalValue(this, key);
}
var internalKey = MemoryEntityService.getKey(key, this.site, this.objType);
var value = getSiteValue(this, internalKey);

process.nextTick(function() { cb(null, value); });
};

Expand All @@ -135,13 +125,11 @@ module.exports = function MemoryEntityServiceModule(pb) {
* @static
* @method getSiteValue
* @param self
* @param key
* @param site
* @param internalKey
* @returns {*}
*/
function getSiteValue(self, key, site) {
function getSiteValue(self, internalKey) {
var rawVal = null;
var internalKey = MemoryEntityService.getKey(key, site);
if (typeof STORAGE[internalKey] !== 'undefined') {
rawVal = STORAGE[internalKey];
}
Expand All @@ -154,18 +142,6 @@ module.exports = function MemoryEntityServiceModule(pb) {
return getCorrectValueField(rawVal, self.valueField);
}

/**
* @private
* @static
* @method getGlobalValue
* @param self
* @param key
* @returns {*}
*/
function getGlobalValue(self, key) {
return getSiteValue(self, key, GLOBAL_SITE);
}

/**
* @private
* @static
Expand Down Expand Up @@ -213,7 +189,7 @@ module.exports = function MemoryEntityServiceModule(pb) {
*/
MemoryEntityService.prototype._set = function(key, value, cb) {
var rawValue = null;
var internalKey = MemoryEntityService.getKey(key, this.site);
var internalKey = MemoryEntityService.getKey(key, this.site, this.objType);
if (STORAGE.hasOwnProperty(internalKey)) {
rawValue = STORAGE[internalKey];
if (this.valueField == null) {
Expand Down Expand Up @@ -251,10 +227,15 @@ module.exports = function MemoryEntityServiceModule(pb) {
key: key,
value: value,
site: this.site,
objType: this.objType,
onlyThisSite: this.onlyThisSite,
keyField: this.keyField,
valueField: this.valueField,
timeout: this.timeout,
ignoreme: true
};
pb.CommandService.getInstance()
.sendCommandToAllGetResponses(MemoryEntityService.getOnChangeType(this.objType), command, util.cb);
.sendCommandToAllGetResponses(MemoryEntityService.getOnChangeType(), command, util.cb);
};

/**
Expand All @@ -270,7 +251,7 @@ module.exports = function MemoryEntityServiceModule(pb) {
}

//check for existing timeout
var internalKey = MemoryEntityService.getKey(key, this.site);
var internalKey = MemoryEntityService.getKey(key, this.site, this.objType);

//now set the timeout if configured to do so
TIMERS[internalKey] = Date.now() + this.timeout;
Expand All @@ -284,7 +265,7 @@ module.exports = function MemoryEntityServiceModule(pb) {
* @param {Function} cb Callback function
*/
MemoryEntityService.prototype.purge = function(key, cb) {
var key = MemoryEntityService.getKey(key, this.site);
var key = MemoryEntityService.getKey(key, this.site, this.objType);
var exists = !!STORAGE[key];
if(exists) {
delete STORAGE[key];
Expand All @@ -293,8 +274,8 @@ module.exports = function MemoryEntityServiceModule(pb) {
process.nextTick(function() { cb(null, exists); });
};

MemoryEntityService.getKey = function(key, site) {
return key + '-' + site;
MemoryEntityService.getKey = function(key, site, objType) {
return key + '-' + site + '-' + objType;
};

/**
Expand All @@ -305,8 +286,8 @@ module.exports = function MemoryEntityServiceModule(pb) {
* @param {String} objType The type of object being referenced
* @return {String} The command type to be registered for
*/
MemoryEntityService.getOnChangeType = function(objType) {
return TYPE + '-' + objType + '-change';
MemoryEntityService.getOnChangeType = function() {
return TYPE + '-change';
};

/**
Expand All @@ -315,10 +296,10 @@ module.exports = function MemoryEntityServiceModule(pb) {
* @static
* @method createChangeHandler
*/
MemoryEntityService.createChangeHandler = function(memoryEntityService) {
return function(command) {
memoryEntityService._set(command.key, command.value, command.site, util.cb);
};
MemoryEntityService.changeHandler = function(command) {

var memoryEntityService = new MemoryEntityService(command);
memoryEntityService._set(command.key, command.value, util.cb);
};

/**
Expand Down Expand Up @@ -364,8 +345,7 @@ module.exports = function MemoryEntityServiceModule(pb) {
}

//clean up by un registering the change handler to prevent memory leaks
pb.CommandService.getInstance().unregisterForType(MemoryEntityService.getOnChangeType(this.objType), this.changeHandler);
this.changeHandler = null;
pb.CommandService.getInstance().unregisterForType(MemoryEntityService.getOnChangeType(), MemoryEntityService.changeHandler);

cb(null, true);
};
Expand Down
4 changes: 2 additions & 2 deletions include/system/command/command_service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ module.exports = function CommandServiceModule(pb) {
*/
CommandService.prototype.registerForType = function (type, handler) {
var result = this.isRegistered(type, handler);
if (result !== CommandService.NOT_FOUND) {
if (result !== CommandService.NOT_FOUND && !!this.registrants[type]) {
return false;
}

Expand Down Expand Up @@ -413,7 +413,7 @@ module.exports = function CommandServiceModule(pb) {
cb = options;
options = {};
}
else if (!pb.ValidationService.isObject(options, false)) {
else if (!pb.ValidationService.isObj(options, false)) {
return cb(new Error('When provided the options parameter must be an object'));
}

Expand Down

0 comments on commit 4bc025b

Please sign in to comment.