Optimiser et sécuriser une transaction PDO

Version imprimable

Bonjour à tous,

Mon but est de recopier les données d'un abonné modèle (id_abo=1) lors de la création d'un nouvel abonné (par exemple id_abo=2).
Je me pose des questions concernant :

le choix de la méthode: N'y a t-il pas plus simple?
la sécurisation: risque d'injection ou de piratage?
l'optimisation: remplacement de la boucle foreach?

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
<?php
function createDefaultRightsGroups($id_abo) {
	global $db;
 
	try {
		$db->beginTransaction();
 
		$select = "
			SELECT designation, comment, access_type, rights
			FROM lst_rights
			WHERE id_abo=1
		;";
		$data = $db->query($select)->fetchAll();
 
		$insert = "
			INSERT IGNORE INTO lst_rights
				(id_abo, designation, comment, access_type, rights
				VALUES(:id_abo, :designation, :comment, :access_type, :rights)
		;";
		$result = $db->prepare($insert);
		$result->bindParam('id_abo', $id_abo);
		foreach ($data as $row) {
			$result->bindParam('designation', $row->designation);
			$result->bindParam('comment', $row->comment);
			$result->bindParam('access_type', $row->access_type);
			$result->bindParam('rights', $row->rights);
			$result->execute();
		}
	}
	catch (PDOException $e) {
		$db->rollBack();
		return false;
	}
	$db->commit();
	return true;	
}
createDefaultRightsGroups(2);
?>

EDIT: Par ailleurs, la colonne 'comment' de la première ligne provoque une erreur de syntaxe 4200/1064.
Contenu de l'objet $row:

Code:

1
2
3
4
5
6
object(stdClass)[4]
  public 'designation' => string 'Superviseur' (length=11)
  public 'comment' => string 'Le groupe superviseur comprend les personnes responsables de l'application.
Ce groupe détient tous les droits.' (length=112)
  public 'access_type' => string '3' (length=1)
  public 'rights' => string '2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,' (length=38)

Salut,

c'est correct, c'est propre, c'est du bon code ;)

Par contre oui tu peux optimiser ton foreach en utilisant fetch à la place de fetchAll :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 $data = $db->query($select)->fetch(); $insert = " INSERT IGNORE INTO lst_rights (id_abo, designation, comment, access_type, rights VALUES(:id_abo, :designation, :comment, :access_type, :rights) ;"; $result = $db->prepare($insert); $result->bindParam('id_abo', $id_abo); $result->bindParam('designation', $data->designation); $result->bindParam('comment', $data->comment); $result->bindParam('access_type', $data->access_type); $result->bindParam('rights', $data->rights); $result->execute();
Je ne sais pas si c'est voulu également ou non mais a priori l'id devrait être auto-incrémenter et donc non renseigné lors de la création du nouvel abonné.

19/07/2017, 15h13
moimp

Citation:

Envoyé par Spartacusply

c'est correct, c'est propre, c'est du bon code ;)

A force de travailler, je commence à être moins mauvais. Merci encore à tous ceux qui, ici ou ailleurs, m'ont aidé à m'améliorer.

Citation:

Envoyé par Spartacusply

Par contre oui tu peux optimiser ton foreach en utilisant fetch à la place de fetchAll :

Ce qui sous entend, que le fasse une boucle pour appeler plusieurs fois la fonction et traiter toutes mes lignes, car j'en ai plusieurs. Je ne pense pas que ce soit mieux.

Citation:

Envoyé par Spartacusply

Je ne sais pas si c'est voulu également ou non mais a priori l'id devrait être auto-incrémenter et donc non renseigné lors de la création du nouvel abonné.

C'est effectivement voulu. Il ne s'agit pas d'une clef primaire ni unique, mais d'un numéro d'abonné.

Il me reste par contre l'erreur #1, EDIT... que je n'arrive pas à résoudre. J'ai essayé un PDO::quote mais ça ne change rien.
RE-EDIT: C'est bon, j'ai trouvé, il manquait la parenthèse de fermeture après la liste de colonnes. Mon problème est donc résolu.