Discussion :

[seyrinian]

Bonjour,

Cela fait plusieurs heures que je me creuse la tête sur un problème d'algorithme, du moins je pense. Lorsque je lance les TU de mon application, la fonction ci-dessous fonctionne 1 fois sur 2 normalement. Je sais que le code en soit est correct et fait ce que j'attends mais la manière dont je l'ai écrit crée parfois des erreurs. Je m'explique. C'est un controller qui vérifie, lors d 'une modification, si certains champs unique ne sont pas déjà existant ou si d'autres données nécessaires sont elle existantes.

Après analyse j'ai remarqué que si je rentre un libellé déjà existant, le temps que la fonction aspectExistByLibelle() qui est une promesse va vérifier le doublon, le code continue et finalement va executer le patchAspect() qui est la fonction demandant au modèle d'update le libelle. Le problème c'est que ce libelle étant unique bah je tombe sur une erreur, qui normalement aurait du être géré par l'aspectExistByLibelle().
Je ne sais pas si je suis très clair, en tout cas je vous joins le code nécessaire à a la compréhension du problème, et si quelqu'un aurait une solution. J'aurai besoin de m'assurer que le patchAspect() attende le retour de aspectExistByLibelle().

Merci par avance pour vos retour,

Seyrinian

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
 
/**
 * Vérifié l'existance d'une aspect via son libelle
 * @param {String} libelle
 * @return {Promise<Boolean|Error>}
 */
const aspectExistByLibelle = function (libelle) {
  return new Promise((resolve, reject) => {
    AspectModel.findByLibelle(libelle).then(aspect => {
      if (aspect) return resolve(true)
      return resolve(false)
    }).catch(err => {
      /* istanbul ignore next */
      return reject(err)
    })
  })
}
 
/**
 * @description Modifier une aspect
 * @param {String} req.params.aspectId identifiant de la aspect
 * @param {Object} req.body propriété à modifier
 * @property {String} req.body.libelle
 * @property {String} req.body.description
 * @property {String} req.body.adjectives
 */
exports.patchById = (req, res, next) => {
  let id = req.params.aspectId
  aspectExistById(id).then(response => {
    if (response) {
      //Check si libelle existe 
      if (req.body.libelle && req.body.libelle !== '') {
        aspectExistByLibelle(req.body.libelle)
          .then((aspectFind) => {
            if (aspectFind) {
              return res.status(400).send({
                error: 'Le libellé existe déjà'
              })
            }
          }).catch(err => {
            /* istanbul ignore next */
            next(err)
          })
      }
      AspectModel.patchAspect(id, req.body)
        .then((aspectPatched) => {
          EventBus.emit(AspectEventBus.updateAspect, aspectPatched)
          return res.status(204).send()
        }).catch((err) => {
          /* istanbul ignore next */
          next(err)
        })
    } else {
      return res.status(404).send({
        error: 'L\' aspect n\'existe pas'
      })
    }
  }).catch(err => {
    /* istanbul ignore next */
    next(err)
  })
}

[NoSmoking]

Bonjour,
regarde du côté de l'opérateur await.

[djedjethai]

Essaies ca .

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
 
 /**
 * Vérifié l'existance d'une aspect via son libelle
 * @param {String} libelle
 * @return {Promise<Boolean|Error>}
 */
const aspectExistByLibelle = function (libelle) {
  return new Promise((resolve, reject) => {
    AspectModel.findByLibelle(libelle).then(aspect => {
      if (aspect) return resolve(true)
      return resolve(false)
    }).catch(err => {
      /* istanbul ignore next */
      return reject(err)
    })  
  })  
}
 
/**
 * @description Modifier une aspect
 * @param {String} req.params.aspectId identifiant de la aspect
 * @param {Object} req.body propriété à modifier
 * @property {String} req.body.libelle
 * @property {String} req.body.description
 * @property {String} req.body.adjectives
 */
exports.patchById = (req, res, next) => {
  let id = req.params.aspectId
  aspectExistById(id).then(response => {
    if (response) {
      //Check si libelle existe 
      if (req.body.libelle && req.body.libelle !== '') {
        aspectExistByLibelle(req.body.libelle)
          .then((aspectFind) => {
            if (aspectFind) {
              return res.status(400).send({
                error: 'Le libellé existe déjà'
              })  
            }   
            AspectModel.patchAspect(id, req.body)
               .then((aspectPatched) => {
                 EventBus.emit(AspectEventBus.updateAspect, aspectPatched)
                 return res.status(204).send()
               }).catch((err) => {
                 /* istanbul ignore next */
                 next(err)
               })  
 
          }).catch(err => {
            /* istanbul ignore next */
            next(err)
          })  
      }   
 
    } else {
          return res.status(404).send({
            error: 'L\' aspect n\'existe pas'
          })
    }
  }).catch(err => {
    /* istanbul ignore next */
    next(err)
  })

[seyrinian]

Pour ta solution ça ne m'arrangerai pas car la zone que je vérifie n'est pas systématiquement présente dans mon patch. Je ne peux donc pas englober le patch dans cette condition. Pour await et async j'ai été voir de ce côté mais mon code ne fonctionne pas lorsque je mets await devant la fonction aspectExisteByLibelle, et async devant la fonction mère.

Je dois mal avoir compris l'utilisation des opérateurs... Il faut bien mettre la fonctionne globale (celle du patch) en async ?

[NoSmoking]

Il faut bien mettre la fonctionne globale (celle du patch) en async ?

La documentation est explicite :

Il ne peut être utilisé qu'au sein d'une fonction asynchrone (définie avec l'instruction async function)

ou encore :

14.7Async Function Definitions.

[djedjethai]

async-await ne changera rien au probleme, cela rendra simplement le code plus lisible. Async-await est du sucre syntaxique.

car la zone que je vérifie n'est pas systématiquement présente dans mon patch.

Dans ce cas la tu rajoutes une condition: if (presente dans le patch) {// code} else {// code} et tu repetes dans chaque cas ton 'AspectModel.patchAspect()' (extenalise de code ds dans une fonction, cela evitera les repetitions)

un truc comme ca (je ne sais pas si la condition match ton code, c'est juste pour l'idee)

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
 
/**
 * Vérifié l'existance d'une aspect via son libelle
 * @param {String} libelle
 * @return {Promise<Boolean|Error>}
 */
const aspectExistByLibelle = function (libelle) {
  return new Promise((resolve, reject) => {
    AspectModel.findByLibelle(libelle).then(aspect => {
      if (aspect) return resolve(true)
      return resolve(false)
    }).catch(err => {
      /* istanbul ignore next */
      return reject(err)
    })
  })
}
 
/**
 * @description Modifier une aspect
 * @param {String} req.params.aspectId identifiant de la aspect
 * @param {Object} req.body propriété à modifier
 * @property {String} req.body.libelle
 * @property {String} req.body.description
 * @property {String} req.body.adjectives
 */
exports.patchById = (req, res, next) => {
  let id = req.params.aspectId
  aspectExistById(id).then(response => {
    if (response) {
      //Check si libelle existe 
      if (req.body.libelle && req.body.libelle !== '') {
          aspectExistByLibelle(req.body.libelle)
            .then((aspectFind) => {
              if (aspectFind) {
                return res.status(400).send({
                  error: 'Le libellé existe déjà'
                })
              }
              AspectModel.patchAspect(id, req.body)
                .then((aspectPatched) => {
                  EventBus.emit(AspectEventBus.updateAspect, aspectPatched)
                  return res.status(204).send()
                }).catch((err) => {
                  /* istanbul ignore next */
                  next(err)
                })
            }).catch(err => {
              /* istanbul ignore next */
              next(err)
            })
        }
      } else {
        AspectModel.patchAspect(id, req.body)
          .then((aspectPatched) => {
            EventBus.emit(AspectEventBus.updateAspect, aspectPatched)
            return res.status(204).send()
          }).catch((err) => {
            /* istanbul ignore next */
            next(err)
          })
      }
    } else {
      return res.status(404).send({
        error: 'L\' aspect n\'existe pas'
      })
    }
  }).catch(err => {
    /* istanbul ignore next */
    next(err)
  })
}

[seyrinian]

Yes en effet la répétition de code était ma première idée mais bon je voulais trouver quelque chose de plus près propre. Enfin je vais faire comme ça tant pis.

J'avais mis ma fonction en async mais en effet ça ne semblait rien changer au problèmes.

Merci à tous en tout cas!

[djedjethai]

Voici ta function refactorise, afin de t'eviter de faire une function separe. Je n'ai pas tester mais avec un peu de chance ca devrait fonctionner.

Bon j'ai finalement fait un test (et vient a l'instant de modif le message), avec ca ca doit fonctionner.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
39
40
41
42
43
44
 
exports.patchById = (req, res, next) => {
  let id = req.params.aspectId
  aspectExistById(id).then(response => {
    if (response) {
 
      	let asyncFunc = new Promise ((resolve, reject) => {
      		if (req.body.libelle && req.body.libelle !== '') {
          		aspectExistByLibelle(req.body.libelle)
            		.then((aspectFind) => {
              			if (aspectFind) {
                			return res.status(400).send({
                  			error: 'Le libellé existe déjà'
                		})
              		}
              		resolve(true);
            	})
 
      		} else {
      			resolve(true);
      		}
  		}) 
 
		asyncFunc.then(readyToPatch => {
			if (readyToPatch) {
				AspectModel.patchAspect(id, req.body)
          			   .then((aspectPatched) => {
            			   EventBus.emit(AspectEventBus.updateAspect, aspectPatched)
            			   return res.status(204).send()
        			})
        	}
		})
		.catch(e => console.log(e)); 
 
  } else {
      return res.status(404).send({
        error: 'L\' aspect n\'existe pas'
      })
    }
  }).catch(err => {
    /* istanbul ignore next */
    next(err)
  })
}

[seyrinian]

En effet ça fonctionne merci.
Après je trouve que le soucis serait si je dois patcher un élement qui possède plusieurs champs à vérifier. Dans ce cas là je vais multiplier mon code de manière exponentielle. Par exemple si je vérifie que mon champs1 est une dupplicate key, je dois gérer le cas Oui/Non, mais si je vérifie si le champs suivants est aussi dupplicateKey bah je dois le vérifier à la fois dans le oui et le non du champs. Je sais pas si je me fait comprendre mais ça devient très vite très lourd.

C'est à croire que l'asynchrone ne va pas du tout si je veux garder en même temps le maximum de contrôle possible sur ce que j'update.

[djedjethai]

Ce que je comprends c'est que tu as des champs (input) a verifier.

Pourquoi multiplier ton code de maniere exponentielle ? Fais des functions, ainsi tu peux les reutiliser.

Si tu dois verifier que tous les champs soient uniques, tu parses l'array ou objet de tes input et verifie (puis retourne ce que tu veux), ce n'est pas asyncrone.

L'asynchronicite n'handicape en rien compare a une technologie synchrone.