Task #1474

Change glpi_configs / glpi_profiles management

Added by moyo almost 13 years ago. Updated almost 9 years ago.

Status:ClosedStart date:08/14/2009
Priority:NormalDue date:
Assignee:webmyster% Done:

100%

Category:Config
Target version:0.85

Description

try to find a solution to permit plugins to add values to config using a simple system.

Idea : a simple table : parameter / value but problem of various datatypes management.

Example of solutions :
  • from fluxbb
    • conf_name VARCHAR NOT NULL DEFAULT '',
    • conf_value TEXT,
    • config cache on file

TODO : search engine for profiles is broken !!

config.diff Magnifier (17.3 KB) webmyster, 11/27/2012 08:18 AM

config.diff Magnifier (18.7 KB) webmyster, 11/27/2012 08:26 AM

profile.diff Magnifier (22.3 KB) webmyster, 11/27/2012 01:48 PM

profile.diff Magnifier (29.9 KB) webmyster, 11/27/2012 01:51 PM

profile.diff Magnifier (34.7 KB) webmyster, 11/28/2012 10:01 AM

profile.diff Magnifier (38.8 KB) webmyster, 11/29/2012 04:04 PM

core.diff Magnifier - Patch for the core (39.6 KB) webmyster, 11/30/2012 11:11 AM

example.diff Magnifier - Patch for example (4.6 KB) webmyster, 11/30/2012 11:11 AM

Associated revisions

Revision 20212
Added by moyo over 9 years ago

[0.85] Change glpi_configs / glpi_profiles management : glpi_configs part : see #1474

Revision 20217
Added by moyo over 9 years ago

[0.85] fix right management see #1474

Revision 20261
Added by moyo over 9 years ago

[0.85] fix install for #1474

Revision 20546
Added by moyo over 9 years ago

[0.85]fix profile search engine see #1474

History

#1 Updated by jmd over 12 years ago

  • Target version changed from 0.78 to 33

#2 Updated by moyo over 11 years ago

  • Target version changed from 33 to 0.85

#3 Updated by ddurieux over 11 years ago

In plugin FusionInventory, we use this table :

CREATE TABLE IF NOT EXISTS `glpi_plugin_fusioninventory_configs` (
  `id` int(1) NOT NULL AUTO_INCREMENT,
  `type` varchar(255) COLLATE utf8_unicode_ci NOT NULL DEFAULT '',
  `value` varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL,
  `plugins_id` int(11) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`),
  UNIQUE KEY `unicity` (`type`,`plugins_id`)
) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci AUTO_INCREMENT=10 ;

For example the version of a plugin :

INSERT INTO `glpi_plugin_fusioninventory_configs`
VALUES (1, 'version', '2.3.0', 6);

With this method, we call for each plugin PluginExampleConfig::showForm() and display it in a tab for each plugin and have centralized configuration of all plugins.

Perhaps we can think to add entity to this config if some values can be used by entity

#4 Updated by webmyster over 9 years ago

David, comments below only concerns the core Config class/glpi_configs table.
Instead of type that does not seems very explicit, i suggest name. What is the purpose of plugins_id ?
The type of the field value should be text, because :
  1. we don't know the size of the type of data (cf. mailing_signature of glpi_configs) ;
  2. we don't have to use the value as a key, because I think we will only search for a name, not a value.

context ?

I also suggest to integrate another field: context that sould be a varchar(244) and sould be a KEY. This is the context of the element: core for the configuration of the GLPI's core and, for instance, fusioninventory, massocimport ...

table structure

As such, the new structure should become :

CREATE TABLE `glpi_configs` (
 `id` int(11) NOT NULL AUTO_INCREMENT,
 `context` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
 `name` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
 `value` text COLLATE utf8_unicode_ci,
 PRIMARY KEY (`id`),
 UNIQUE KEY `unicity` (`name`,`context`)
);

The current version of GLPI should be:

INSERT INTO `glpi_configs` (`context`, `name`, `value`) VALUES ('core', 'version', '0.85')

Whereas version of FusionInventory should be:
INSERT INTO `glpi_configs` (`context`, `name`, `value`) VALUES ('fusioninventory', 'version', '2.1.1');

Or:

INSERT INTO `glpi_configs` (`context`, `name`, `value`) VALUES ('plugin:fusioninventory', 'version', '2.1.1');

Implementation

I suggest to include methods inside Config class to search values regarding a given context to simplify treatment for the plugins.

$names  = array('version', 'list_limit', `show_jobs_at_login`);
$values = Config::getValues('core', $names);

That should return :
$values = array('version' => '0.85', 'list_limit' => '15', 'show_jobs_at_login' => '0');

Data type ?

We can also integrate a field value_type that is an enum : string, integer, float, boolean .... With such system, Config::getValues will return :

$values = array('version' => '0.85', 'list_limit' => 15, 'show_jobs_at_login' => false);

But I'm not sure that it is very usefull.

entity ?

Integration of the entity seems a good idea. But we have to care about some attributs that MUST NOT be override, for instance: version.

Application to glpi_profiles

I think that we should define an equivalent mechanism for glpi_profiles to allow the plugins to use Session::haveRight() with their own rights.

Integration

We have to be very carefull with this new schema. Among others, the install/update.php should be updated to integrate this: line 568, we should test if version field is existing inside glpi_configs table.

Thus, although we will care on compatibility, this major config should destabilize the main trunk. So I suggest to do it just after forking trunk to 0.84. While freezing any other development on the main trunk.

#5 Updated by webmyster over 9 years ago

I suggest a first "working" patch for glpi_config.
There is several check we must do to unsure everything is OK ...

#6 Updated by webmyster over 9 years ago

Little mistake in previous patch ...

#7 Updated by webmyster over 9 years ago

New version of the patch augmented with same behavious for config and profile.
Missing documentation and should be validated by bug report !

#8 Updated by webmyster over 9 years ago

Grrr !
Same patch + missing migration file (update_084_085.php)

#9 Updated by moyo over 9 years ago

L'ensemble me semble plutôt cohérent.
Il y aura certainement quelques petites choses à ajouter.
par exemple le contrôle des profiles ayant moins de droit que celui qui est utilisé. Avec une table de droits ca va être plus compliqué (Profile::getUnderActiveProfileRetrictRequest)

#10 Updated by webmyster over 9 years ago

Sans oublier que cette évolution permettra d'intégrer les droits de plugins dans le tronc commun.
Mais je pense que cela ne perturbera pas le fonctionnement de Profile::getUnderActiveProfileRetrictRequest.

#11 Updated by webmyster over 9 years ago

Même patch, mais amélioré pour gérer Profile::getUnderActiveProfileRetrictRequest.

J'ai eu un petit soucis avec le champs create_ticket_on_login qui n'est pas inteprété comme un droit lors de la migration (type différent de char(1)). Est-ce bien le cas ?

Par ailleurs, je ne suis pas sûr que l'ensemble des modifications nécessaires à la création d'une migration 0.84->0.85 soient intégrées. Il faudrait créer une migration propre puis importer les éléments du patch.

#12 Updated by moyo over 9 years ago

webmyster wrote:

Même patch, mais amélioré pour gérer Profile::getUnderActiveProfileRetrictRequest.

J'ai eu un petit soucis avec le champs create_ticket_on_login qui n'est pas inteprété comme un droit lors de la migration (type différent de char(1)). Est-ce bien le cas ?

C'est un bool non ? On peut le convertir en char si besoin.

Par ailleurs, je ne suis pas sûr que l'ensemble des modifications nécessaires à la création d'une migration 0.84->0.85 soient intégrées. Il faudrait créer une migration propre puis importer les éléments du patch.

pas de problème la dessus.

#13 Updated by webmyster over 9 years ago

moyo wrote:

webmyster wrote:

Même patch, mais amélioré pour gérer Profile::getUnderActiveProfileRetrictRequest.

J'ai eu un petit soucis avec le champs create_ticket_on_login qui n'est pas inteprété comme un droit lors de la migration (type différent de char(1)). Est-ce bien le cas ?

C'est un bool non ? On peut le convertir en char si besoin.

On peut le transformer en char sans problème (ajout de deux lignes dans la migration). Je pose la question au niveau conceptuel : ce champs doit-il être considéré comme un droit, ou est-ce un attribut du profile ?

Par ailleurs, le patch transforme les droits NULL en ''. Il me semble que ces deux valeurs sont similaires. Ce n'est pas un impératif, mais cela simplifie les requêtes SQL en évitant de dissocier le cas IS NULL des cas IN ('', ...).

#14 Updated by webmyster over 9 years ago

Enhancement of the patch: better deal with creation/destruction of profiles.
Ensure all rights are available for a given profile.

#15 Updated by webmyster over 9 years ago

Little update of the patch (core.diff) + patch for example plugin (example.diff).
We can enhance everything, but that is a starter element

#16 Updated by moyo over 9 years ago

  • Subject changed from Change glpi_configs management to Change glpi_configs / glpi_profiles management

#17 Updated by webmyster over 9 years ago

A possible improvement : we can merge all configurations inside the $CFG_GLPI configuration variable during the startup.
We can create a 'context' entry inside $CFG_GLPI tat contains all contexts (for the plugins) at startup.
For instance for the IPAM plugin, there will be $CFG_GLPI['context']['plugin:IPAM:Radius']['format'] entry.

Another further improvement, will be to merge everything and straightly use the context :
  • $CFG_GLPI['networkport_types'] becomes $CFG_GLPI['core']['networkport_types'] ;
  • $CFG_GLPI['context']['plugin:IPAM:Radius']['format'] becomes $CFG_GLPI['plugin:IPAM:Radius']['format'].
The key element will be the possibility to contextualise the core config elements:
  • $CFG_GLPI['ajax_min_textsearch_load'], $CFG_GLPI['ajax_buffertime_load'] ... becomes $CFG_GLPI['core:ajax']['min_textsearch_load'], $CFG_GLPI['core:ajax']['buffertime_load'] ;
  • $CFG_GLPI['date_format'], $CFG_GLPI['names_format'] ... becomes $CFG_GLPI['core:format']['date'], $CFG_GLPI['core:format']['names'] ;
  • ...

But we have to care about some plugins that may have very lots of configuration features. That can create a bottleneck to get a value.

#18 Updated by moyo over 9 years ago

Will see tomorrow but maybe change char(1) field to text one to permit to integrate all others items to glpi_profilerights.
This work will change all the values : Rights_management (must only change upgrade process)

#19 Updated by moyo over 9 years ago

  • % Done changed from 0 to 80

#20 Updated by moyo about 9 years ago

  • % Done changed from 80 to 100

#21 Updated by yllen about 9 years ago

  • Status changed from New to Resolved

#22 Updated by moyo almost 9 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF