PROBLEMA
El problema (siempre lo es) es de asincronía. Todo proceso de lectura / escritura en base de datos es un proceso asíncrono, por lo tanto, los datos devueltos por la consulta no estarán disponibles inmediatamente al llamar al método de la consulta, sino que serán pasados como argumento a la función callback
que utilicemos para procesarlos.
Esto quiere decir que, dada la naturaleza de Javascript, al llamar al método findById(id).exec(callback)
, el mismo no devolverá un resultado, sino que ejecutará la siguiente instrucción en el orden que aparezca, la cual en este caso es un console.log(productoObtenido)
. Y dado que el mismo es undefined
eso es lo que muestra en consola.
Por otro lado, estás intentando hacer una serie de operaciones con elementos de una lista, lo cual si bien parece lo correcto, dada la naturaleza de la información, no es lo adecuado.
Actualmente tienes:
carrito.productos.forEach(producto => {
let id = producto._id;
const productoObtenido = Producto.findById(id)
.exec((err, productoDB) => {
if (err) {
return false;
}
if (!productoDB) {
return false;
}
console.log(productoDB);
return productoDB;
});
console.log(productoObtenido);
});
Lo cual no es muy eficiente desde el punto de vista de la cantidad de consultas que se realizan a la BD. Si tienes 100 productos, haces 100 peticiones para obtener los productos, cosa que podrías lograr con ¡¡una sola consulta!!.
SOLUCIÓN
Una de las soluciones es escribir el resto de tu lógica dentro de la función callback
, es lo que haré en mi respuesta, además te mostraré la forma de realizar una sola consulta sin importar la cantidad de elementos que estemos consultando.
Lo primero será realizar la consulta, la cual puedes hacer así:
let addCarrito = (req, res) => {
let carrito = new Carrito(req.body); // <- no necesitas escribir los valores, luego lo explico
//obtenemos la lista de ids de cada producto
let productosId = carrito.productos.map(producto => producto._id);
Producto.find({_id: {$in: productosId}}).exec((error, productos) => { ... });
// ...
Como puedes observar he realizado algunos cambios de cara a mejorar la cantidad de código innecesario que escribes.
Cuando creas una instancia de un Modelo de Mongoose, no importa si el objeto del que sacas la información tiene más o menos campos que el modelo, se llenarán todos los que contenga el objeto origen y los que no existan serán undefined
y los que sobren no se tomarán en cuenta. Eso te ahorra escribir cada campo de req.body
al crear la instancia del modelo y se resume en un simple: new Carrito(req.body);
Por otro lado, puedes obtener una lista de todos los productos de la colección de productos que coincidan con los _id
que están en la lista creada (productosId
). Puedes ver la notación usada al llamar al método map()
del tipo Array
. Se le conoce como programación funcional.
Para consultar en una sola llamada por todos los productos, usamos el operador de consulta $in
de MongoDB, que como se observa recibe una lista y busca todos los elementos de la colección que tengan un campo _id
coincidente con algún valor de la lista.
Si se consigue algún documento se añadirá a la lista de resultados, si no se consigue ningún elemento, la lista será devuelta vacía.
Ahora que tenemos la lista de productos, podemos proceder a salvar el carrito con los productos de la lista, o devolver un mensaje al cliente indicando que ocurrió un error o que la lista de productos está vacía (no se encontraron coincidencias). Este proceso debe hacerse dentro de la función callback
que devuelve los resultados de esta primera consulta:
let addCarrito = (req, res) => {
let carrito = new Carrito(req.body);
let productosId = carrito.productos.map(producto => producto._id);
Producto.find({_id: {$in: productosId}}, (error, productos) => {
if(error) {
return res.status(500).json({
message: 'Ocurrió un error'
});
}
if(!productos.length) {
return res.status(404).json({
message: 'No se encontró ningún producto'
});
}
let cantidadProductosOriginal = carrito.productos.length;
carrito.productos = [ ...productos]; // <- reemplazamos los productos del carrito por la lista de productos existente obtenida
carrito.save((error, saved) => {
if (error) {
return res.status(500).json({
ok: false,
error: 'Ocurrió un error' // nunca muestres el error interno
});
}
return res.json({
ok: true,
carrito: saved._id,
productosSalvados: productos.length,
productosOriginales: cantidadProductosOriginal
});
}); // finaliza carrito.save
}); // finaliza Producto.find
} // finaliza addCarrito
Una de las cosas importantes que debes notar es que cada vez que llamo al método de respuesta al cliente (res
), lo hago usando la sentencia return
. Esto es porque de esa forma doy por finalizado el proceso de la función y la misma ya no se ejecutará a partir de ese punto. Esto nos evitará los errores de tipo Cannot set headers before they are send
, que son tan comunes porque se olvida este pequeño detalle.
También es bueno tomar en cuenta que nunca se debe enviar el mensaje de error interno del servidor al cliente. Si MongoDB nos devuelve un error, es para nosotros y no para el cliente.
Por último, puedes ver que agregué 2 valores a la respuesta cuando el carrito ha sido salvado, esto con la idea de informar cuantos productos, de los enviados originalmente, fueron salvados efectivamente en el carrito.
Espero que esto te ayude a resolver el problema.
callback
en el métodoexec()
deProducto.findById()
, entonces se devuelveundefined
, ya que es un proceso asíncrono. Tu variableproductoDB
sólo existe dentro de esa funcióncallback
y no podrás sacarla de allí. Lo que debes hacer es ecribir el resto de tu lógica dentro de esacallback
. Saludosundefined
, entonces pense que podía llegar a ser por eso. Nunca hice algo así como me decís, de escribir la lógica dentro delcallback
, me podrías dar un ejemplo? Gracias por la respuesta!