De thin controllers y detached entities

62 views
Skip to first unread message

Francesc Rosàs

unread,
Apr 24, 2012, 8:51:25 AM4/24/12
to symfony_...@googlegroups.com
Hola symfoneros, estoy trabajando en un proyecto en Symfony2 + Doctrine2 y los controllers se me están engordando ya demasiado con tareas que entiendo que pertenecen al modelo.

Había pensado que para aligerarlos podría empezar con extraer todo el código que pertenece al modelo en forma de servicios. Así si por ejemplo tengo un action del controller que (pongamos que estamos en Twitter) crea un tuit a partir de un formulario y notifica a todos los seguidores, la idea sería crear un servicio (e.g. twitter.tweets) con un método publish($tweet). El controller seguiría instanciando y trabajando con un objeto Tweet pero no tendría ningún $em->flush() ni nada por el estilo.

Las ventajas vendrían a ser estas:
  • El controller hace sólo de pasarela entre el usuario y el modelo
  • El modelo se puede reutilizar en otros contextos (en procesos batch, por ejemplo)
  • Puedo cambiar la implementación del modelo sin afectar a los controllers (e.g. pasar de usar una base de datos a un web service)
Uno de los aspectos que me genera más dudas es la forma de encapsular correctamente los cambios sobre las entities. Doctrine por defecto recuerda los cambios que se hacen en las entidades y ahí pierdo la encapsulación que me da el servicio ya que al hacer un $em->flush() se aplicarían todos los cambios que se hayan podido hacer en cualquier entidad y sin mucho control sobre qué quiero aplicar y qué no en la base de datos.

Para evitar esto había pensado en algo así:

class TweetsService
{
    function publish(Tweet $tweet)
    {
        $this->getEntityManager()->clear();
        $this->getEntityManager()->merge($tweet);
        $this->getEntityManager()->flush();

        $this->notifyFollowers($tweet);
    }
}

Básicamente olvida todos los cambios hechos en las entidades (el clear() hace un detach() de todas ellas) y aplica sólo los que nos interesan.

Cómo veis esta aproximación? Me estoy liando sólo? Va a afectar mucho en el rendimiento? Con las pruebas que he hecho hasta ahora la cosa funciona pero no me gustaría encontrarme con problemas más adelante.

Un saludo.

Xavi Montero

unread,
Apr 24, 2012, 9:21:37 AM4/24/12
to symfony_...@googlegroups.com
Hola,

Toda la aproximación a "separar" del controller es buena.
Pero está mal el planteamiento de hacer el "clear". Si una cosa no la quieres persistir, no toques esa clase.

El motivo de NO hacer el "clear" es el siguiente: Imagina que en vez de "servicio al que llamas" fuera "evento que recoges", esto es el controlador dice "->dispatch('tweet', ...)" y el TweetBundle recogiera el evento para grabarlo en la BBDD... imagina que OTRO bundle inertara un hook delante con más prioridad (por ejemplo MonitorBundle) que monitorizara si en el tweet hay una palabra malsonante para en todo caso "interceptar" su posting y sustituir esa palabra malsonante por "[eliminado]". Además el MonitorBundle decidiera poner en una tabla aparte las ocurrencias de palabras malsonantes para que el adminsitrador del sistema pueda hacer algo.

Si tú en el TweetBundle haces un "clear", te estás cargando sin ninguna clase de autorización, que OTRO haga sus cosas. Vamos, como si viene alguien que no conoces y se pone a limpiarte tu coche... por mucho que siempre habías querido limpiarlo y nunca te ponías a ello seguro que tu reacción es "¿oye, pero tú qué haces con mi coche?" y él te dice "es que si está limpio, mejor, así si tengo que verte por la calle no me molestará a la vista"... sin duda él no tiene por qué limpiarte nada si no te conoce por mucho que él quiera eso.

Así pues yo me limitaría a tocar mis clases, y a hacer el flush al final.

Si al final de la llamada se han hecho dos o tres flushes no pasa nada.

Y si lo que quieres es que un objeto no mute en la BBDD, no le toques sus propiedades. Debes pensar en objetos de doctrine como si fueran "objetos persistentes" (caray qué coincidencia con el nombre del método). Y aquí lo dels "escacs" va a misa: "Fitxa tocada, fitxa moguda". Si necesitas "tocar" un objeto temporalmente y no persistir, seguramente es que necesitas una clase "proxy" de modelo que "esconda" una clase Doctrine dentro y que el modelo ofrezca su propia API.

Saludos!
Xavi.

Ricard Clau

unread,
Apr 24, 2012, 10:07:00 AM4/24/12
to symfony_...@googlegroups.com
Hola

Estoy de acuerdo con Xavi respecto al clear y con los dos en cuanto a la aproximación. Parece que escribas más pero es el camino a seguir: skinny controllers y lógica en capa de servicios. Así se simplifica el desarrollo de APIs, de múltiples front-ends (web, móvil, webapp), etc...

Pero ojo con hacer flush más de una vez ya que los flush envían los cambios a la BBDD y se inicia una nueva transacción. Y eso puede no ser lo que queremos.

Francesc Rosàs

unread,
Apr 24, 2012, 10:51:58 AM4/24/12
to symfony_...@googlegroups.com
Jajaja muy bueno el ejemplo del desconocido que te limpia el coche, aunque no me importaría acostumbrarme a ello ;)

Estoy de acuerdo que hacer un clear() es algo drástico y puede que una buena fuente de efectos colaterales (léase bugs) difíciles de detectar. 

Por otro lado un flush() me parece igual de drástico si no sabes qué hay en el unit of work actual. Siguiendo con tu ejemplo, sería el flush() del listener del MonitorBundle el que persistiría todos los cambios y el listener del TweetBundle se quedaría sin poder realizar más cambios antes de persistirlos.

Por último no veo porqué el caso que comentas debería fallar. Te pico un ejemplo sin usar eventos para hacerlos más corto y fácil de seguir:

class TweetsService
{
    function publish(Tweet $tweet)
    {
        $this->replaceBadWords($tweet);

        $this->getEntityManager()->clear();
        $this->getEntityManager()->merge($tweet);
        $this->getEntityManager()->flush();

        $this->notifyFollowers($tweet);
    }

    private replaceBadWords(Tweet $tweet)
    {
        $badWordsFound = $this->doReplaceBadWords($tweet);

        $this->getEntityManager()->clear();
        foreach ($badWordsFound as $word) {
            $badWord = new TweetBadWord($tweet, $word);
            $this->getEntityManager()->merge($badWord);
        }
        $this->getEntityManager()->flush();
    }
}

Salut!

El dimarts 24 d’abril de 2012 15:21:37 UTC+2, Xavi Montero va escriure:

El dimarts 24 d’abril de 2012 15:21:37 UTC+2, Xavi Montero va escriure:
El dimarts 24 d’abril de 2012 15:21:37 UTC+2, Xavi Montero va escriure:

Ronny López

unread,
Apr 24, 2012, 12:13:14 PM4/24/12
to symfony_...@googlegroups.com
Hola, 

En teoría la interfaz que exponga la capa de servicio debe estar desligada totalmente de la persistencia.

En el caso que planteas, a los clientes del TweetService no les concierne como este servicio gestione la persistencia de los objetos que maneja internamente. 

Piensa por ejemplo, que hoy se pueden estar persistiendo los tweets en MySQL, a través de Doctrine ORM, pero que mañana puedes decidir cambiar a MongoDB a través de Doctrine ODM, o a Redis a través de un cliente de Redis propio.

Entonces, si el servicio maneja la persistencia de forma interna, ninguno de sus clientes se verá afectado ante un cambio de este tipo.

Unos buenos ejemplos a seguir en este tipo de diseño de "modelo de datos abstracto", son el FOSUserBundle y el FOSCommentBundle.

Aquí se interactua con los objetos del modelo a través de un "Manager", el CommentManager por ejemplo. El CommentManager tiene una interfaz bien defina, la cual es totalmente independiente de la persistencia: https://github.com/FriendsOfSymfony/FOSCommentBundle/blob/master/Model/CommentManagerInterface.php

Entonces, existe una implementación abstracta https://github.com/FriendsOfSymfony/FOSCommentBundle/blob/master/Model/CommentManager.php e implementaciones especializadas en diferentes tipos de persistencia, por ejemplo, una basada en Doctrine: https://github.com/FriendsOfSymfony/FOSCommentBundle/blob/master/Entity/CommentManager.php

Es cierto que este tipo de diseño require más trabajo de diseño y programación, pero la robustez y desacoplamiento que aporta a una aplicación medianamente compleja es importante. 

Saludos,
Ronny.

Francesc Rosàs

unread,
Apr 26, 2012, 1:01:16 PM4/26/12
to symfony_...@googlegroups.com
Ronny, sí de momento estoy liado quitando la lógica del negocio de los controladores. El siguiente paso seguramente sería el que comentas, extraer la persistencia en forma de repositorios (más que nada para facilitar el testeo de estos servicios).

Y gracias por los ejemplos, la verdad es que cuesta encontrar los que pasan del simple "controlador manipula entidades".

theUniC

unread,
Apr 26, 2012, 1:33:22 PM4/26/12
to symfony_...@googlegroups.com
Hola,

La verdad es que la persistencia tampoco debería ir ligada a la capa de repositorios, pues los repositorios solo deberían responsabilizarse de la extracción de entidades del domain model -- http://martinfowler.com/eaaCatalog/repository.html

En todo caso, tal cómo comenta Ronny, deberían tener su propia capa!

Un saludo!
Christian.

Francesc Rosàs

unread,
Apr 26, 2012, 1:36:50 PM4/26/12
to symfony_...@googlegroups.com
Vale, después de mirarme un poco más el tema veo que por mucho que funcione el código anterior va en contra de los principios de cómo funciona un ORM (es que uno está acostumbrado a tratar con active records!)

Pero aquí me surgen más dudas. Si acepto que un flush() puede persistir todos los cambios hechos en cualquier entidad entonces debo asegurarme que estas entidades estén en un estado válido. La forma que veo cómo Symfony2 plantea la validación es permitiendo que las propiedades de una entidad "se coman" cualquier valor (sea válido o no) y luego validar la entidad cuando sea necesario. Esta es la forma que usa el componente Form para validar los cambios de cara al usuario, pero qué pasa si los cambios en las entidades no vienen de un formulario? Tengo que validar también las entidades antes de hacer el flush?

De momento es lo que estoy haciendo (https://gist.github.com/2501178) pero se me hace estraño tener que implementar algo me parece tan básico (y sin tener el cuenta que si la entidad es modificada mediante un formulario la validación se hace por duplicado).

Por otro lado, no se supone que una buena encapsulación de los objetos no debería permitir que estos estuvieran en un estado no válido?

Francesc Rosàs

unread,
Apr 26, 2012, 1:47:36 PM4/26/12
to symfony_...@googlegroups.com
No estoy seguro de a qué te refieres por "extracción de entidades del domain model". Si te refieres a sólo obtener esas entidades del data store, según el propio artículo que comentas un repositorio sirve tanto para "meter" como para "sacar":

Conceptually, a Repository encapsulates the set of objects persisted in a data store and the operations performed over them, providing a more object-oriented view of the persistence layer.

Y para dejarlo más claro:

Objects can be added to and removed from the Repository, as they can from a simple collection of objects, and the mapping code encapsulated by the Repository will carry out the appropriate operations behind the scenes.
 
Vamos, como tener un array de entidades persistido y adaptado a tus necesidades :)

theUniC

unread,
Apr 26, 2012, 2:41:12 PM4/26/12
to symfony_...@googlegroups.com
Un repository te sirve para consultar datos de tu fuente de datos, no para persistirlos. Entiendo que no se refiere a "meter" (por persistir) de forma explícita, sino que el propio repository lo hace de manera transparente cuándo se ejecuta una consulta contra ese repository para solicitar datos.

Para no "cargar de responsabilidad" a los repositories, habría que desacoplar la persistencia de los propios repositories y añadir una capa más que cumpla con esa responsabilidad.

Por otro lado, el input hay que validarlo siempre y venga de dónde venga. Aquí pasa un poco cómo en la cuestión de los repositories. Y es que el patrón ActiveRecord, al menos la mayor parte de implementaciones que he visto, suelen acoplar bastante la validación del input con la propia persistencia. Y eso es un mal vicio. La responsabilidad de que el input sea válido no debería recaer en la capa de persistencia.

El componente Validate, a través del service "validate", se encarga de validar las entidades (que pueden estar vinculadas a un formulario o no) según las reglas definidas. El componente Form ya de por si, y transparentemente, usa este servicio para validar la entidad que tenga asociada.


Un saludo!
Christian.

Francesc Rosàs

unread,
Apr 26, 2012, 3:07:44 PM4/26/12
to symfony_...@googlegroups.com
El 26 d’abril de 2012 20:41, theUniC <the...@gmail.com> ha escrit:
Un repository te sirve para consultar datos de tu fuente de datos, no para persistirlos. Entiendo que no se refiere a "meter" (por persistir) de forma explícita, sino que el propio repository lo hace de manera transparente cuándo se ejecuta una consulta contra ese repository para solicitar datos.

Para no "cargar de responsabilidad" a los repositories, habría que desacoplar la persistencia de los propios repositories y añadir una capa más que cumpla con esa responsabilidad.

Creo que el Sr. Fowler y yo seguimos discrepando ;)

A no ser que estemos hablando de lo mismo con palabras diferentes. Así lo veo yo:

Controllers -> Servicios -> Repositorios -> Data mapper (e.g. un ORM) -> Data store
 
Por otro lado, el input hay que validarlo siempre y venga de dónde venga. Aquí pasa un poco cómo en la cuestión de los repositories. Y es que el patrón ActiveRecord, al menos la mayor parte de implementaciones que he visto, suelen acoplar bastante la validación del input con la propia persistencia. Y eso es un mal vicio. La responsabilidad de que el input sea válido no debería recaer en la capa de persistencia.

El componente Validate, a través del service "validate", se encarga de validar las entidades (que pueden estar vinculadas a un formulario o no) según las reglas definidas. El componente Form ya de por si, y transparentemente, usa este servicio para validar la entidad que tenga asociada.


Es por eso que me extraña que la validación no sea una parte intrínseca de las entidades. Algo como:

$user->setEmail('foo'); // Exception al canto!

En cambio estás "obligado" a validar ese $user cada vez que lo recibes como argumento de un método. Entiendo que haciéndolo así simplifica bastante la lógica de setear las propiedades de una entidad desde un formulario pero complica el resto de capas.

theUniC

unread,
Apr 26, 2012, 5:31:25 PM4/26/12
to symfony_...@googlegroups.com
Sí, discrepamos. Un Repository que además de consultar datos se encargue de persistencia, no me suena muy SOLID. Me viene más a la cabeza un servicio de persistencia que no acoplarlo a los repositories. Pero claro, eso acaba yendo a gustos. Yo personalmente, me siento más cómodo con todo bien desacoplado y las responsabilidades de cada capa bien definidas :D

Por otro lado, cargar a los setters con validaciones para mi gusto es sobrecargar la propia entidad y pervertirla. Mejor tener un servicio de validación que se encargue de validar todo el input. Realmente no estás "obligado", pues todo el input debería pasar por un único punto de entrada, que se encargue mediante los correspondientes servicios de validarlo para que el resto de la aplicación pueda trabajar de manera segura con esos datos.

Un saludo!
Christian.
Reply all
Reply to author
Forward
0 new messages