Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add external data manager #246

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 184 additions & 0 deletions src/Payutc/Bom/ExternalData.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
<?php

namespace Payutc\Bom;
use \Payutc\Db\Dbal;
use \Payutc\Bom\User;
use \Payutc\Exception\ExternalDataException;
use \Payutc\Log;

class ExternalData {

protected static function addSelectConditions($qb, $fun_id, $usr_id = null) {
$qb->from('t_external_data_exd', 'exd')
->where('fun_id = :fun_id')
->andWhere('exd_removed is NULL')
->setParameter('fun_id', $fun_id);

if ($usr_id === null) {
$qb->andWhere('usr_id is NULL');
}
else {
$qb->andWhere('usr_id = :usr_id')
->setParameter('usr_id', $usr_id);
}
}

protected static function insert($fun_id, $key, $val, $usr_id = null) {
$conn = Dbal::conn();
$a = $conn->insert('t_external_data_exd', array(
'fun_id' => $fun_id,
'usr_id' => $usr_id,
'exd_key' => $key,
'exd_val' => $val,
'exd_inserted' => 'NOW()'));
}

/**
* @param $fun_id
* @param $key
* @param $usr_id
* @param $full true to get full record, false to get only the value
* @param $for_update true to do a SELECT ... FOR UPDATE statement
*/
public static function get($fun_id, $key, $usr_id = null, $full = false, $for_update = false) {
static::checkKey($key);
$qb = Dbal::createQueryBuilder();
$qb->select($full ? '*' : 'exd_val');
if ($for_update) {
$qb->forUpdate();
}
static::addSelectConditions($qb, $fun_id, $usr_id);
$qb->andWhere('exd_key = :exd_key')
->setParameter('exd_key', $key);
$res = $qb->execute()->fetch();
if ($res === false) {
throw new ExternalDataException("Not found : fun=$fun_id, key=$key, usr_id=$usr_id", 404);
}
return $full ? $res : $res['exd_val'];
}

/**
*
* @param $fun_id
* @param $key
* @param $val
* @param $usr_id
*/
public static function set($fun_id, $key, $val, $usr_id = null) {
static::checkKey($key);
$conn = Dbal::conn();

$conn->beginTransaction();
try {

$affected_rows = static::del($fun_id, $key, $usr_id);

if ($affected_rows == 0) {
Log::info("create new external data ($fun_id, $key, $val, $usr_id)");
}

// insert the new record
static::insert($fun_id, $key, $val, $usr_id);

$conn->commit();
}
catch (Exception $e) {
$conn->rollback();
throw $e;
}
}

/**
* @param $fun_id
* @param $key
* @param $usr_id
*/
public static function del($fun_id, $key, $usr_id = null) {
static::checkKey($key);

$qb = Dbal::createQueryBuilder();
$qb->update('t_external_data_exd', 'exd')
->where('fun_id = :fun_id')
->setParameter('fun_id', $fun_id)
->where('exd_key = :key')
->setParameter('key', $key)
->andWhere('exd_removed is NULL')
->set('exd_removed', 'NOW()');

if ($usr_id === null) {
$qb->andWhere('usr_id is NULL');
}
else {
$qb->andWhere('usr_id = :usr_id')
->setParameter('usr_id', $usr_id);
}

$affected_rows = $qb->execute();

if ($affected_rows > 1) {
Log::warning("multiple rows has been deleted ($fun_id, $key, $usr_id)"); // TODO
}
// one row affected, the record was already existing
else if ($affected_rows == 1) {

}
// 0 row affected, the record does not exist
else {

}

return $affected_rows;
}


/**
* Available transformations ($func):
* array('$inc'=> (int))
* array('$dec'=> (int))
*
* @param $fun_id
* @param $key
* @param $fun the transformation to apply
* @param $usr_id
* @return the new value
*/
public static function transform($fun_id, $key, array $func, $usr_id = null) {
static::checkKey($key);
$conn = Dbal::conn();

$conn->beginTransaction();
try {
$full = false;
$res = self::get($fun_id, $key, $usr_id, $full, true);
$transform = key($func);
$value = reset($func);
switch ($transform) {
case '$dec':
$value = -(int)$value;
break;
case '$inc':
$res = (int)$res + (int)$value;
break;
default:
throw new ExternalDataException('Invalid transformation');
break;
}
self::set($fun_id, $key, $res, $usr_id);
$conn->commit();
}
catch (Exception $e) {
$conn->rollback();
Log::error("Impossible d\'effectuer la transformation ($fun_id, $key, $func, $usr_id) ".$e->getMessage());
}

return $res;
}

protected static function checkKey($key) {
$pattern = '/^[a-zA-Z0-9_-]+$/';
if(preg_match($pattern, $key) === 0) {
throw new ExternalDataException("Invalid key, allowed char are [a-zA-Z0-9_-]", 442);
}
}
}

6 changes: 6 additions & 0 deletions src/Payutc/Exception/ExternalDataException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

namespace Payutc\Exception;

class ExternalDataException extends PayutcException {}

1 change: 1 addition & 0 deletions src/Payutc/Mapping/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Services {
'ADMINRIGHT',
'BLOCKED',
'GESARTICLE',
'DATAADMIN',
'PAYLINE',
'RELOAD',
'MYACCOUNT',
Expand Down
2 changes: 1 addition & 1 deletion src/Payutc/Migrations/Version20130829233731.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function up(Schema $schema)

public function down(Schema $schema)
{
$this->addSql("ALTER TABLE `t_purchase_pur2`
$this->addSql("ALTER TABLE `t_purchase_pur`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ça devrait être dans une autre PR, ça non ?

DROP `pur_qte`,
DROP `pur_unit_price`;");
}
Expand Down
39 changes: 39 additions & 0 deletions src/Payutc/Migrations/Version20131002042353.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

namespace Payutc\Migrations;

use Doctrine\DBAL\Migrations\AbstractMigration,
Doctrine\DBAL\Schema\Schema;

/**
* Auto-generated Migration: Please modify to your need!
*/
class Version20131002042353 extends AbstractMigration
{
public function up(Schema $schema)
{
$this->addSql("
CREATE TABLE IF NOT EXISTS `t_external_data_exd` (
`exd_id` int(11) NOT NULL AUTO_INCREMENT,
`fun_id` int(11) unsigned NOT NULL,
`usr_id` int(11) unsigned,
`exd_key` varchar(128) NOT NULL,
`exd_val` varchar(1024) NOT NULL,
`exd_inserted` datetime NOT NULL,
`exd_removed` datetime,
PRIMARY KEY (`exd_id`),
KEY `fun_id` (`fun_id`),
KEY `usr_id` (`usr_id`),
KEY `exd_key` (`exd_key`(127))
) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ;");
$this->addSql("
ALTER TABLE `t_external_data_exd`
ADD CONSTRAINT `t_external_data_exd_ibfk_2` FOREIGN KEY (`usr_id`) REFERENCES `ts_user_usr` (`usr_id`),
ADD CONSTRAINT `t_external_data_exd_ibfk_1` FOREIGN KEY (`fun_id`) REFERENCES `t_fundation_fun` (`fun_id`)");
}

public function down(Schema $schema)
{
$this->addSql("DROP TABLE `t_external_data_exd`;");
}
}
126 changes: 126 additions & 0 deletions src/Payutc/Service/DATAADMIN.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<?php


namespace Payutc\Service;


use \Payutc\Config;
use \Payutc\Log;
use \Payutc\Bom\ExternalData;
use \Payutc\Exception\ExternalDataException;
use \Payutc\Bom\User;

class DATAADMIN extends \ServiceBase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il manque le delete


/**
* valid inputs:
* $usr = array('login' => 'trecouvr');
* $usr = array('badge' => 'ABCDEABCDE');
* $usr = array('id' => 1);
* $usr = null
*
* output:
* id : int or null
*/
protected function convertUsrArg($usr) {
if ($usr !== null) {
if (isset($usr['badge'])) {
$u = User::getUserFromBadge($usr['badge']);
$usr = $u->getId();
}
else if (isset($usr['login'])) {
$u = new User($usr['login']);
$usr = $u->getId();
}
else if (isset($usr['id'])) {
$usr = $usr['id'];

}
}
return $usr;
}

protected function get($fun_id, $key, $usr = null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Du coup il faut que $usr soit un array. Il n'y avait pas un bug dans payutc-json-client qui empêchait l'envoi d'array en paramètre ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cette méthode est protected, c'est celle qui est utilisée par les autres fonctions (c'est celle aussi qui fait le checkright)

$this->checkRight(true, true, true, $fun_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cette gestion des droits, implique que l'user actuellement logué ait le droit d'utiliser le service.

Ce qui implique que dans un cas comme mozart, il faillent donner les droits à tout les vendeurs.
Si tu veux faire un client qui permet aux utilisateurs de voir les données les concernants, il faut donner les droits à tout les utilisateurs d’accéder au datamanager etc...

Bref je pense que l'accès à l'application est suffisant, demander les droits à l'utilisateur est contraignant.
(EX: Si une application billetterie veut stocker des données pour l'utilisateur actuellement logué, il faut donner à tout les utilisateurs le droit DATAADMIN)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pour moi ce n'est pas l'utilisateur qui touche ces données, c'est la fondation qui met des données sur un utilisateur.

Je peux enlever le check user, du coup on checkerai juste que l'application a le droit de toucher des donnés sur la fondation ca me parait raisonable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, du coup fait le.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toujours en attente

$usr = $this->convertUsrArg($usr);
return ExternalData::get($fun_id, $key, $usr);
}

protected function set($fun_id, $key, $val, $usr = null) {
$this->checkRight(true, true, true, $fun_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem pour les droits

++ Ne crois-tu pas que l'on voudrait dissocier les droits en lecture et ceux en écriture ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On devrait surement les dossicier oui, j'avoue que c'est plus un acces a une ressource du coup ca a du sens de dissocier ecriture/lecture, le probleme c'est que notre system n'est pas fait comme ca et ca m'embete de rajouter une table de droit.
Dans notre system on donne des droits sur des services, pas sur des resources, du coup si on veut dissocier écriture/lecture on fait deux services.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je pense qu'on peut accepter une première implémentation qui donne à une/des application(s) les droits sur toutes les données d'une fundation.

Dans un second temps il faudra peut être isoler les données de chaque application. Mais bon vu qu'il n'y a pas de methode pour récupérer la liste des "clefs", chaque application peut générer un token long pour préfixer ces valeurs et ainsi les protéger des autres applications. (Et si une application s'amuse à buteforcer toutes les clefs possible ça apparaitra très vite dans les logs et il sera simple de remonter à elle).

Du coup je suis prêt à valider le service dans la forme, ou l'on donne le droit à une application a accéder (lecture et ecriture) a toutes les clefs/valeurs d'une fundations.

PS: Il n'y a donc pas de droit sur le user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nan on accepte pas, c'est pas bon, on a trouvé une meilleur méthode sur gdoc, et je prefererai vraiment que ça soit géré en mongodb.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bah dans ce cas, il faut fermer la PR... (Mais franchement je doute que l'on ai mongodb à court terme...)
En plus ça va compléxifier le projet (je pense par exemple à l'icam qui est entrain de travailler activement à l'implémentation de payutc chez eux... ils auront jamais mongodb)

$usr = $this->convertUsrArg($usr);
return ExternalData::set($fun_id, $key, $val, $usr);
}

protected function del($fun_id, $key, $usr = null) {
$this->checkRight(true, true, true, $fun_id);
$usr = $this->convertUsrArg($usr);
return ExternalData::del($fun_id, $key, $usr);
}

protected function transform($fun_id, $key, $func, $usr = null) {
$this->checkRight(true, true, true, $fun_id);
$usr = $this->convertUsrArg($usr);
$func = json_decode($func, true);
if ($func === null) {
Log::warning("'$func' is not a valid format for transform method");
throw new ExternalDataException('Invalid format');
}
return ExternalData::transform($fun_id, $key, $func, $usr);
}


/// FUN DATA

public function getFunData($fun_id, $key) {
return $this->get($fun_id, $key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il n'y a aucune gestion des droits sur les méthodes concernant les data de la fundation. N'importe qui peut y accéder et les modifier.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Voir commentaire ci-dessus, les droits sont gérés dans les méthodes "outils" (get, set, del).

}

public function setFunData($fun_id, $key, $val) {
return $this->set($fun_id, $key, $val);
}

public function delFunData($fun_id, $key) {
return $this->del($fun_id, $key);
}

public function transformFunData($fun_id, $key, $func) {
return $this->transform($fun_id, $key, $func);
}

/// USR DATA

public function getUsrDataByLogin($fun_id, $login, $key) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est pas cohérent avec le reste de ton api... Tu as un set/get/del qui sait gérer Login/Id/Badge... Pourquoi rajoutes-tu les méthodes séparément ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Voir commentaire ci dessus, ce sont ces méthodes qui sont accessibles depuis le client, les autres sont des méthode "outil".

return $this->get($fun_id, $key, array('login'=>$login));
}

public function setUsrDataByLogin($fun_id, $login, $key, $val) {
return $this->set($fun_id, $key, $val, array('login'=>$login));
}

public function getUsrDataByBadge($fun_id, $badge, $key) {
return $this->get($fun_id, $key, array('badge'=>$badge));
}

public function setUsrDataByBadge($fun_id, $badge, $key, $val) {
return $this->set($fun_id, $key, $val, array('badge'=>$badge));
}

public function delUsrDataByLogin($fun_id, $login, $key) {
return $this->del($fun_id, $key, array('login'=>$login));
}

public function delUsrDataByBadge($fun_id, $badge, $key) {
return $this->del($fun_id, $key, array('badge'=>$badge));
}

public function transformUsrDataByLogin($fun_id, $login, $key, $func) {
return $this->transform($fun_id, $key, $func, array('login'=>$login));
}

public function transformUsrDataByBadge($fun_id, $badge, $key, $func) {
return $this->transform($fun_id, $key, $func, array('badge'=>$login));
}
}


Loading