Servicios (Si es transaccional, no utilizar flush: true)

[Ismael]

Buenas,

He visto el video: Greach 2013 - Todo lo que me hubiera gustado saber cuando empecé a desarrollar con grails

El caso es que si quito el flush:true de algún método transaccional que luego accedo a los datos calculados, no existen y por lo tanto me falla otra parte de la aplicación. Si no es recomendado el flush:true entonces como asegurarme de que esos datos ya son posibles de recuperar desde otra parte de la aplicación.

Saludos y gracias.

[Iván López]

Por alusiones ya que la charla es mía... ;-)

Lo primero decir que eso fue con Grails 1.3.7 y ha llovido bastante desde aquel entonces, así que muchas de las cosas que dije en su momento puede que ahora no sean verdad. De hecho acabo de ver ese fragmento del video y no es cierto lo que digo. Hacer flush:true en un servicio transaccional no implica que estemos comiteando los cambios y no podamos hacer rollback de los mismos (puesto que la transacción sigue abierta). En lugar de explicarlo yo, ya se encargó en su momento de hacer Mario, así que te remito a estos post en su antiguo blog:

http://desmontandojava.blogspot.com.es/2013/08/grails-pitfalls-dont-do-flushtrue-when.html

http://desmontandojava.blogspot.com.es/2013/08/grails-pitfalls-dont-do-flushtrue-when_20.html

En cualquier caso, respecto al flush:true, yo en general he seguido sin utilizarlo en todos estos años. Si todo el código está en servicios (no en controllers con @Transactional) en mi caso siempre veo esas modificaciones en el mismo flujo de ejecución. Donde si necesito algún flush ocasionalmente es en los tests para que algunos datos que creo ahí se vean a la hora de ejecutar el código. Pero en todo caso los flush van en los tests y no en el código de producción.

Si tienes algún caso concreto con código lo podemos mirar con más calma, pero después de más de 4 años sigo pensando lo mismo que en la charla. No usar flush:true, al menos "por defecto y a lo loco".

Saludos, Iván.

--
@ilopmar

--
Has recibido este mensaje porque estás suscrito al grupo "grailsEnCastellano" de Grupos de Google.
Para cancelar la suscripción a este grupo y dejar de recibir sus mensajes, envía un correo electrónico a grailsencastellano+unsub...@googlegroups.com.
Para publicar en este grupo, envía un correo electrónico a grailsencastellano@googlegroups.com.
Visita este grupo en https://groups.google.com/group/grailsencastellano.
Para acceder a más opciones, visita https://groups.google.com/d/optout.

[Ismael]

Muchas gracias por las aclaraciones.

Pues Iván tengo un proyecto hecho en grails 2.3.11, que tampoco es de lo más actual.

Y el código es el siguiente:

Tengo un método en un servicio que realiza un calculo de horario próximo:

/**
 * Obtiene cual es la siguiente clase que tiene que dar es decir la unidad (conjunto de profesor, asignatura y grupo)
 * @param profesor
 * @return
 */
Horario obtenerProximoHorario(Profesor profesor) {
    Date hoy = new Date()
    Date diaCalculado, diaCalculadoMasCercano, diaCalculadoFestivo
    List<Date> festivoList = festivoService.obtenerFestivos()

    def horarios = profesor?.horarios?.findAll{ !it.dias.empty }

    horarios?.each{ horarioInstance ->
        List<DiaHorario> diaHorarioList = horarioInstance.dias.sort{ it.diaSemana }

        diaHorarioList.eachWithIndex{ DiaHorario diaHorarioInstance, int counter ->

            diaCalculado = fechaService.calcularSiguienteClase(hoy,
                    diaHorarioInstance.diaSemana?.numero,
                    diaHorarioInstance.tramoHorario?.endHours,
                    diaHorarioInstance.tramoHorario?.endMins)
            diaCalculadoFestivo = diaCalculado.clone()

            while(diaCalculadoFestivo.clearTime() in festivoList){
                // es un día festivo calculamos el siguiente día que no sea festivo
                diaCalculadoFestivo = fechaService.calcularSiguienteClase(diaCalculado,
                        diaHorarioInstance.diaSemana?.numero,
                        diaHorarioInstance.tramoHorario?.endHours,
                        diaHorarioInstance.tramoHorario?.endMins)
                if(diaCalculado == diaCalculadoFestivo){
                    // en caso de que el calculado vuelva a ser el mismo día porque no haya pasado la hora de la clase
                    diaCalculadoFestivo = fechaService.calcularSiguienteClase(diaCalculadoFestivo + 1,
                            diaHorarioInstance.diaSemana?.numero,
                            diaHorarioInstance.tramoHorario?.endHours,
                            diaHorarioInstance.tramoHorario?.endMins)
                    diaCalculado = diaCalculadoFestivo.clone()
                }
                else{
                    diaCalculado = diaCalculadoFestivo.clone()
                }
            }

            if(counter == 0) {
                // primer iteración
                diaCalculadoMasCercano = diaCalculado
                diaHorarioInstance.siguienteDiaFin = diaCalculado
                horarioInstance.diaHorarioSiguiente = diaHorarioInstance
            }

            if (diaCalculado?.after(hoy) && diaCalculado?.before(diaCalculadoMasCercano)) {
                diaCalculadoMasCercano = diaCalculado
                diaHorarioInstance.siguienteDiaFin = diaCalculado
                horarioInstance.diaHorarioSiguiente = diaHorarioInstance
            }
        }

        horarioInstance.save()
    }

    if(horarios && !horarios.empty){
        horarios.sort { it.diaHorarioSiguiente?.siguienteDiaFin }
        horarios.first()
    }
    else{
        horarios
    }
}

La línea que he puesto en amarillo es la que me afecta si la pongo el flush:true o no.

Puesto que cuando entro en la siguiente método que tengo en el controller que cargo la información de:

diaHorarioSiguiente

No esta calculado.

Este es el código de ese método que crea el modelo de datos para las vistas de pasar faltas:

protected createMultiEditModel(mdl = [:]){
    MultiplePasarListaCommand command = mdl.command
    List <TramoHorario> tramoHorarioList = []

    Horario horarioInstance = Horario.read(session.idHorarioCargado)
    if (horarioInstance){
        command?.horario = command?.horario ?: horarioInstance
        // por defecto el diaHorarioSiguiente y obtenemos "fecha" y "tramoHorario"
        command?.fecha = command?.fecha ?: horarioInstance?.diaHorarioSiguiente?.siguienteDiaFin
        command?.tramoHorario = command?.tramoHorario ?: horarioInstance?.diaHorarioSiguiente?.tramoHorario

        Evaluacion.list().each{
            command?.evaluacion = fechaService.perteneceAlIntervalo(command?.fecha, it.inicio, it.fin) ? it : null
        }

        TipoDiaSemana tipoDiaSemana
        if (!command?.fecha){
            // si la primera vez que acceder por defecto cojemos el dia siguiente que hay clases
            tipoDiaSemana = horarioInstance?.diaHorarioSiguiente?.diaSemana
            // obtenemos los tramos horarios de la asignatura para ese día por si hay más de un tramo horario
            tramoHorarioList = tramoHorarioService.obtenerTramoHorarios(horarioInstance, tipoDiaSemana)
        }
        else{
            Integer dayOfWeek = fechaService.getDayOfWeek(command?.fecha)
            tipoDiaSemana = TipoDiaSemana.values()[dayOfWeek-1]
            // obtenemos los tramos horarios de la asignatura para ese día por si hay más de un tramo horario
            tramoHorarioList = tramoHorarioService.obtenerTramoHorarios(horarioInstance, tipoDiaSemana)
        }

        params.offset = params.int('offset') ?: 0
        params.sort = params['sort'] ?: 'ordenarNombre'
        params.order = params['order'] ?: 'asc'

        command?.alumnos = command?.alumnos ?: alumnoService.getAllStudentsInHorario(horarioInstance, params)

        FaltaAsistencia faltaAsistenciaInstance = FaltaAsistencia.get(command?.faltaSeleccionada)
        if(!faltaAsistenciaInstance){
            // faltas de ese dia en ese tramo horario
            List<FaltaAsistencia> faltaAsistenciaList = faltaAsistenciaService
                    .obtenerFaltasDiaHora(horarioInstance, command?.fecha, command?.tramoHorario)
            command?.faltas = faltaAsistenciaList.collect{ new SinglePasarListaCommand(it) }
        }
        else{
            SinglePasarListaCommand faltaInstance = command?.faltas.find{ it.id == faltaAsistenciaInstance?.id }
            command?.faltas.remove(faltaInstance)
            command?.faltas << new SinglePasarListaCommand(faltaAsistenciaInstance)
        }
    }

    mdl << [tramoHorarioList: tramoHorarioList]

    render view: 'multiEdit', model:mdl
}

Saludos y gracias, ya me comentas que opinas del código. Me gusta aprender de mis errores.

[Iván López]

Hola,

El código es demasiado largo y complejo como para poder analizarlo correctamente. En cualquier caso, un par de sugerencias (por favor no te lo tomes a mal, son totalmente constructivas):

- Mueve toda la lógica a servicios. Un controller de 50 líneas está haciendo demasiadas cosas que no debería hacer. Para mi un controller lo único que tiene que hacer es usar un command object para los parámetros, validarlos y dar error si no son correctos y si lo son, llamar a un servicio pasando los parámetros necesarios y responder con la respuesta. Todo eso no debería ocupar más de 10-15 líneas. Por supuesto que hay casos concretos, pero mi regla general es que si un controller es tan largo, algo estoy haciendo mal.

- Haciendo lo anterior consigues que testear un controller sea muy sencillo. En tu caso anterior necesitas al menos 7-8 tests para cubrir todo el código (porque hay demasiados if-else)

- Abuso de save-navigation. Es algo que veo constantemente en mi día a día que hace el código muy complicado de leer y entender. Cada vez que ponemos un '?' para evitar null pointer deberíamos pensar si realmente eso puede ser nulo o no. En caso de que pueda serlo, probablemente es mejor controlarlo de otra forma y devolver un error en lugar de arrastrar el ? por todo el método. Yo he llegado a ver código como este: params?.foo, request?.xxx, User?.findBy... y obviamente ni params, request o la clase User van a ser null NUNCA.

- Modificación de parámetros en el command. Para mi los parámetros en un command deberían ser inmutables. Si necesitamos modificarlos en función de otros parámetros probablemente lo estamos haciendo mal. Lo que yo hago es crearme helpers en la clase del command object que me devuelven los parámetros ya transformados. Así, en lugar de hacer comprobaciones y modificar cmd.foo en función de otros parámetros, creo el método cmd.fetchFoo() en el que pongo toda la lógica para devolver el valor apropiado de "foo". Esto tiene 3 ventajas: me permite testear todo de manera aislada en el command object, no modifico los parámetros y el controller queda muchísimo más limpio porque no hago ninguna validación ni modificación de parámetros (simplemente "extraigo" lo que necesito del command).

Saludos, Iván.

--
@ilopmar

--

[Ismael]

Buenos días,

Iván muchas gracias por lo comentarios. Los tengo en cuenta.

A ver si me puede orientar como puedo rebajar el volumen de líneas de la función protected que uso para crear el modelo de datos. En realidad creo que mis action en mis controller son pequeños quitando la función que crea el modelo.

Ejemplo de action de un controller:

def multiSave(MultiplePasarListaCommand command){
    FaltaAsistencia.withTransaction { status ->
        try{
            Boolean okValidated = (command.validate() && !command.hasErrors())
            if (okValidated) {
                faltaAsistenciaService.saveFaltas(command)
                flash.message = message(code: 'default.updated.message',
                        args: [message(code: 'faltaAsistencia.label', default: 'Faltas asistencia'), command.toString()])
                createMultiEditModel(command: command)
            }
            else {
                log.error command
                log.error command.errors
                status.setRollbackOnly()
                createMultiEditModel(command: command)
            }
        } catch (e){
            status.setRollbackOnly()
            log.error e
            createMultiEditModel(command: command)
        }
    }
}

Como debo abstraer para rebajar el número de lineas. Debo usar más nameQueries, o más servicios, me orientas. Y comentarte que sigo sin saber como puedo evitar usar el flush:true.

Saludos y muchísimas gracias, si viviera en Madrid me encantaría ir a las reuniones que hacéis allí en Madrid.

[Paul]

Hola Iván, si tienes algun proyecto subido a git o algo parecido para que podamos tener una guía de como lo haces, nos serviría de ejemplo para saber como se tienen que hacer las cosas :)

Gracias

[Iván López]

Ismael,

En mi opinión ese controller tiene un problema. Estás haciendo "withTransaction" y si hay algún error haces el rollback manualmente de la transacción. Si tienes que hacer algo como eso o poner @Transactional en un controller, entonces esa lógica debería estar en un servicio transaccional. En él es todo más fácil porque no te tienes que preocupar de esos problemas, si hay una excepción automáticamente se hará rollback de la transacción.

Otra cosa respecto al command object. Cuando lo usas como parámetro en una acción de un controller, Grails ejecuta el .validate() por ti, así que no es necesario que lo ejecutes después manualmente, sólo deberías comprobar si tiene errores.

Saludos, Iván.

--
@ilopmar

--

Has recibido este mensaje porque estás suscrito al grupo "grailsEnCastellano" de Grupos de Google.

Para cancelar la suscripción a este grupo y dejar de recibir sus mensajes, envía un correo electrónico a grailsencastellano+unsubscribe@googlegroups.com.

[Iván López]

Paul,

Me temo que no tengo ningún proyecto público en el que haya cosas como estas. Son proyectos de clientes y son privados.

Como punto de partida os recomiendo: http://guides.grails.org/. Cada lunes publicamos una guía nueva con ejemplos prácticos de distintos aspectos del framework. Las hay desde las más básicas sobre cómo crear vuestra primera aplicación Grails a otras más avanzadas como Server send events, multi-tenancy con GORM, pasando por integraciones con Angular o React. 

Os recomiendo que le echeis un vistazo porque en ellas seguimos las buenas prácticas que recomendamos.

Saludos, Iván.

--
@ilopmar

2017-08-21 9:28 GMT+02:00 Paul <polos...@gmail.com>:

Hola Iván, si tienes algun proyecto subido a git o algo parecido para que podamos tener una guía de como lo haces, nos serviría de ejemplo para saber como se tienen que hacer las cosas :)

Gracias

--