-
Notifications
You must be signed in to change notification settings - Fork 16
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
Changes from all commits
e323b0b
156fa7e
eb7732a
6661ed6
17dc208
902e59f
230353f
e976f08
99a39fb
e7d2f6b
3d5539f
d2c846c
385e707
d75f6d9
666d742
0b4a005
42f893a
50b3c04
fe4e40e
7ee0eb1
59fd505
2fe4dec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<?php | ||
|
||
namespace Payutc\Exception; | ||
|
||
class ExternalDataException extends PayutcException {} | ||
|
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`;"); | ||
} | ||
} |
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Bref je pense que l'accès à l'application est suffisant, demander les droits à l'utilisateur est contraignant. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oui, du coup fait le. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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...) |
||
$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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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 ?