[SPIP Zone] Commandes et référence unique : proposition d'évolution avec risque de bug

Archives Zone
1

Hello,

ce message concerne le plugin commandes et une proposition de modification de la définition de la référence, pour s'assurer que ça ne casse rien chez personne.

Problème :
* la référence d'une commande est censée être unique
* on a pas de contrainte sur le champ 'reference' de la table spip_commandes pour le garantir
* la méthode de calcul repose est simplement un time(), donc 2 commandes passées dans la même seconde auront la même référence : bonjour le bazar

Avant que ça n'arrive à quelqu'un je propose les modifications qui suivent :

1/ situation actuelle
la référence commande est determinée avant insertion en base, dans la fonction commande_inserer() :
https://zone.spip.org/trac/spip-zone/browser/plugins/commandes/trunk/action/editer_commande.php#L73

on appelle la fonction
commandes_reference() surchargeable avec pour seul argument id_auteur
https://zone.spip.org/trac/spip-zone/browser/plugins/commandes/trunk/inc/commandes_reference.php

la fonction se contente d'un time() comme seule référence, d'où le problème

2/ la correction proposée :

la fonction commandes_reference() serait appelée après insertion en base, avant appel du pipeline post_insertion genre ici
https://zone.spip.org/trac/spip-zone/browser/plugins/commandes/trunk/action/editer_commande.php#L114

elle prendrait en second argument $id_commande, donc l'id unique de la commande en base

la fonction
commandes_reference($id_auteur,$id_commande)
génèrerait une référence de la forme

YYYYMMDDIIIIII

avec IIIIII l'id_commande sur 6 chiffres (000001 pour id_commande=1)

# Avantages :
* on assure l'unicité des références commande
* la référence contient explicitement la date, ce qui facilite la gestion
* on passe des références actuelles sur 10 chiffres à des références sur 14 chiffres mini, donc on reste incrémental : les nouvelles références se placeront bien postérieurement après les anciennes, y compris dans le cas d'un tri alphabetique puisque commençant par 2 au lieu de 1 actuellement

# Inconvénient :
* si quelqu'un utilise le pipeline pre_insertion sur les commandes et s'attend à y retrouver une référence de commande, ça ne sera plus le cas, la référence n'apparaissant que dans le pipeline post_insertion.

Donc petit risque de bug.
Est-ce que ça concerne des utilisateurs ici ?

* on a toujours pas de contrainte unique sur le champ reference de la table, car une telle contrainte empêcherait l'insertion de 2 commandes avec reference=''

On peut le gérer dans commandes_inserer() en générant un uniqid temporaire en guise de reference pour l'insertion et en re-essayent 3 fois en cas de collision (ce qui serait vraiment un comble de malchance), mais là ça casserait potentiellement tout autre code qui fait des sql_insertq brutaux

Qu'en pensez-vous ?
Go en version simple ?
En version avec contrainte d'unicité sur le champ reference ?
Pasgo pasgo ?

--
Cédric

2

Je dirais déjà go en version simple, en changeant le format et le moment. Pour l'unicité je ne sais pas, je ne me visualise pas trop. Mais bon ça pourrait être fait aussi à priori.

--
RastaPopoulos

3

Je dirais même plus : go go go !

Le 28/04/2017 à 16:17, RastaPopoulos a écrit :

Je dirais déjà go en version simple, en changeant le format et le moment. Pour l'unicité je ne sais pas, je ne me visualise pas trop. Mais bon ça pourrait être fait aussi à priori.

4

A l'epreuve du code, je suis obligé de changer la signature de la fonction commandes_reference en cassant la compat dessus car id_commande devient obligatoire
J'y vais quand même ?

Index: action/editer_commande.php

--- action/editer_commande.php (revision 103930)
+++ action/editer_commande.php (working copy)
@@ -81,7 +81,7 @@
         include_spip('inc/autoriser');
         if (autoriser('commander','',0,$id_auteur)){
                 // La date de tout de suite
- $champs['date'] = date('Y-m-d H:i:s');
+ $champs['date'] = date('Y-m-d H:i:s', $_SERVER['REQUEST_TIME']);

                 // Le statut en cours par défaut
                 if (!isset($champs['statut'])) {
@@ -88,12 +88,6 @@
                         $champs['statut'] = 'encours';
                 }

- // La référence si elle n'est pas déjà donnée
- if (!isset($champs['reference'])) {
- $fonction_reference = charger_fonction('commandes_reference', 'inc/');
- $champs['reference'] = $fonction_reference($champs['id_auteur']);
- }
-
                 // Envoyer aux plugins avant insertion
                 $champs = pipeline('pre_insertion',
                         array(
@@ -103,7 +97,7 @@
                                 'data' => $champs
                         )
                 );
-
+
                 // Si on veut insérer des échéances et que ce n'est pas déjà sérialisé, on sérialise TOUJOURS ce champ
                 if (isset($champs['echeances']) and @unserialize($champs['echeances']) === false) {
                         $champs['echeances'] = serialize($champs['echeances']);
@@ -112,6 +106,13 @@
                 // Insérer l'objet
                 $id_commande = sql_insertq("spip_commandes", $champs);

+ // La référence si elle n'est pas déjà donnée : on attend d'avoir l'id_commande pour pouvoir generer un numero unique
+ if (!isset($champs['reference'])) {
+ $commandes_reference = charger_fonction('commandes_reference', 'inc/');
+ $champs['reference'] = $commandes_reference($id_auteur, $id_commande);
+ sql_updateq('spip_commandes', array('reference' => $champs['reference']), 'id_commande=' . intval($id_commande));
+ }
+
                 // Envoyer aux plugins après insertion
                 pipeline('post_insertion',
                         array(
Index: inc/commandes_reference.php

--- inc/commandes_reference.php (revision 103858)
+++ inc/commandes_reference.php (working copy)
@@ -22,13 +22,23 @@
   * $fonction_reference = charger_fonction('commandes_reference', 'inc/');
   * ```
   *
- * @param string $id_auteur
- * (inutilisé) identifiant de l'auteur
- * @return int
- * Nombre de secondes écoulées depuis le 1er janvier 1970
+ * @param int $id_commande
+ * @param int $id_auteur
+ * (optionnel) identifiant de l'auteur
+ * @return string
+ * reference de la commande
  **/
-function inc_commandes_reference_dist($id_auteur=0){
- return time();
+function inc_commandes_reference_dist($id_commande, $id_auteur=0){
+
+ if ($date = sql_getfetsel('date', 'spip_commandes', 'id_commande='.intval($id_commande))) {
+ $t = strtotime($date);
+ }
+ else {
+ $t = $_SERVER['REQUEST_TIME'];
+ }
+
+ // format YYYYMMDDNNNNNN
+ $reference = date('Ymd', $t) . str_pad(intval($id_commande), 6, '0', STR_PAD_LEFT);
+
+ return $reference;
  }

--
Cédric

Charles Razack a écrit :

Je dirais même plus : go go go !

Le 28/04/2017 à 16:17, RastaPopoulos a écrit :

Je dirais déjà go en version simple, en changeant le format et le
moment. Pour l'unicité je ne sais pas, je ne me visualise pas trop.
Mais bon ça pourrait être fait aussi à priori.

----
spip-zone@rezo.net - http://listes.rezo.net/mailman/listinfo/spip-zone

5

Le 28/04/2017 à 16:02, Cédric Morin a écrit :

Est-ce que ça concerne des utilisateurs ici ?

ça ne devrait pas poser de problème pour la boutique https://www.aphg.fr

--
Yanic Gornet - 06 08 60 46 81

6

C'est donc envoyé dans

Alea jacta est

--
Cédric

Cédric Morin a écrit :

A l'epreuve du code, je suis obligé de changer la signature de la
fonction commandes_reference en cassant la compat dessus car id_commande
devient obligatoire
J'y vais quand même ?

Index: action/editer_commande.php

--- action/editer_commande.php (revision 103930)
+++ action/editer_commande.php (working copy)
@@ -81,7 +81,7 @@
include_spip('inc/autoriser');
if (autoriser('commander','',0,$id_auteur)){
// La date de tout de suite
- $champs['date'] = date('Y-m-d H:i:s');
+ $champs['date'] = date('Y-m-d H:i:s', $_SERVER['REQUEST_TIME']);

// Le statut en cours par défaut
if (!isset($champs['statut'])) {
@@ -88,12 +88,6 @@
$champs['statut'] = 'encours';
}

- // La référence si elle n'est pas déjà donnée
- if (!isset($champs['reference'])) {
- $fonction_reference = charger_fonction('commandes_reference', 'inc/');
- $champs['reference'] = $fonction_reference($champs['id_auteur']);
- }
-
// Envoyer aux plugins avant insertion
$champs = pipeline('pre_insertion',
array(
@@ -103,7 +97,7 @@
'data' => $champs
)
);
-
+
// Si on veut insérer des échéances et que ce n'est pas déjÃ
sérialisé, on sérialise TOUJOURS ce champ
if (isset($champs['echeances']) and @unserialize($champs['echeances'])
=== false) {
$champs['echeances'] = serialize($champs['echeances']);
@@ -112,6 +106,13 @@
// Insérer l'objet
$id_commande = sql_insertq("spip_commandes", $champs);

+ // La référence si elle n'est pas déjà donnée : on attend d'avoir
l'id_commande pour pouvoir generer un numero unique
+ if (!isset($champs['reference'])) {
+ $commandes_reference = charger_fonction('commandes_reference', 'inc/');
+ $champs['reference'] = $commandes_reference($id_auteur, $id_commande);
+ sql_updateq('spip_commandes', array('reference' =>
$champs['reference']), 'id_commande=' . intval($id_commande));
+ }
+
// Envoyer aux plugins après insertion
pipeline('post_insertion',
array(
Index: inc/commandes_reference.php

--- inc/commandes_reference.php (revision 103858)
+++ inc/commandes_reference.php (working copy)
@@ -22,13 +22,23 @@
* $fonction_reference = charger_fonction('commandes_reference', 'inc/');
* ```
*
- * @param string $id_auteur
- * (inutilisé) identifiant de l'auteur
- * @return int
- * Nombre de secondes écoulées depuis le 1er janvier 1970
+ * @param int $id_commande
+ * @param int $id_auteur
+ * (optionnel) identifiant de l'auteur
+ * @return string
+ * reference de la commande
**/
-function inc_commandes_reference_dist($id_auteur=0){
- return time();
+function inc_commandes_reference_dist($id_commande, $id_auteur=0){
+
+ if ($date = sql_getfetsel('date', 'spip_commandes',
'id_commande='.intval($id_commande))) {
+ $t = strtotime($date);
+ }
+ else {
+ $t = $_SERVER['REQUEST_TIME'];
+ }
+
+ // format YYYYMMDDNNNNNN
+ $reference = date('Ymd', $t) . str_pad(intval($id_commande), 6, '0',
STR_PAD_LEFT);
+
+ return $reference;
}

7

Bien le bonjour,

Après avoir galéré à ne pas comprendre pourquoi mon numéro de commande était toujours le même je me suis rendu compte que :

dans action/editer_commande.php, ligne 112 on a : $champs['reference'] = $commandes_reference($id_auteur, $id_commande);
Quand la fonction commandes_reference demande les variables dans l'ordre inverse : function inc_commandes_reference_dist($id_commande, $id_auteur=0)

Bien cordialement,

Chourak

----- Mail original -----
De: "Cédric Morin" <cedric@yterium.com>
À: spip-zone@rezo.net
Envoyé: Mercredi 17 Mai 2017 13:08:18
Objet: Re: [SPIP Zone] Commandes et référence unique : proposition d'évolution avec risque de bug

C'est donc envoyé dans

Alea jacta est

--
Cédric

Cédric Morin a écrit :

A l'epreuve du code, je suis obligé de changer la signature de la
fonction commandes_reference en cassant la compat dessus car id_commande
devient obligatoire
J'y vais quand même ?

Index: action/editer_commande.php

--- action/editer_commande.php (revision 103930)
+++ action/editer_commande.php (working copy)
@@ -81,7 +81,7 @@
include_spip('inc/autoriser');
if (autoriser('commander','',0,$id_auteur)){
// La date de tout de suite
- $champs['date'] = date('Y-m-d H:i:s');
+ $champs['date'] = date('Y-m-d H:i:s', $_SERVER['REQUEST_TIME']);

// Le statut en cours par défaut
if (!isset($champs['statut'])) {
@@ -88,12 +88,6 @@
$champs['statut'] = 'encours';
}

- // La référence si elle n'est pas déjà donnée
- if (!isset($champs['reference'])) {
- $fonction_reference = charger_fonction('commandes_reference', 'inc/');
- $champs['reference'] = $fonction_reference($champs['id_auteur']);
- }
-
// Envoyer aux plugins avant insertion
$champs = pipeline('pre_insertion',
array(
@@ -103,7 +97,7 @@
'data' => $champs
)
);
-
+
// Si on veut insérer des échéances et que ce n'est pas déjÃ
sérialisé, on sérialise TOUJOURS ce champ
if (isset($champs['echeances']) and @unserialize($champs['echeances'])
=== false) {
$champs['echeances'] = serialize($champs['echeances']);
@@ -112,6 +106,13 @@
// Insérer l'objet
$id_commande = sql_insertq("spip_commandes", $champs);

+ // La référence si elle n'est pas déjà donnée : on attend d'avoir
l'id_commande pour pouvoir generer un numero unique
+ if (!isset($champs['reference'])) {
+ $commandes_reference = charger_fonction('commandes_reference', 'inc/');
+ $champs['reference'] = $commandes_reference($id_auteur, $id_commande);
+ sql_updateq('spip_commandes', array('reference' =>
$champs['reference']), 'id_commande=' . intval($id_commande));
+ }
+
// Envoyer aux plugins après insertion
pipeline('post_insertion',
array(
Index: inc/commandes_reference.php

--- inc/commandes_reference.php (revision 103858)
+++ inc/commandes_reference.php (working copy)
@@ -22,13 +22,23 @@
* $fonction_reference = charger_fonction('commandes_reference', 'inc/');
* ```
*
- * @param string $id_auteur
- * (inutilisé) identifiant de l'auteur
- * @return int
- * Nombre de secondes écoulées depuis le 1er janvier 1970
+ * @param int $id_commande
+ * @param int $id_auteur
+ * (optionnel) identifiant de l'auteur
+ * @return string
+ * reference de la commande
**/
-function inc_commandes_reference_dist($id_auteur=0){
- return time();
+function inc_commandes_reference_dist($id_commande, $id_auteur=0){
+
+ if ($date = sql_getfetsel('date', 'spip_commandes',
'id_commande='.intval($id_commande))) {
+ $t = strtotime($date);
+ }
+ else {
+ $t = $_SERVER['REQUEST_TIME'];
+ }
+
+ // format YYYYMMDDNNNNNN
+ $reference = date('Ymd', $t) . str_pad(intval($id_commande), 6, '0',
STR_PAD_LEFT);
+
+ return $reference;
}

----
spip-zone@rezo.net - http://listes.rezo.net/mailman/listinfo/spip-zone