Grails Anti-Pattern: Locally Optimized Dynamic Finders Everywhere

The context

Grails makes it very easy to persist and find stuff using domain classes. It uses GORM (Grails’ Object Relational Mapping) under the hood, which by default uses Hibernate to map domain classes to tables in a database. Powerful stuff and makes it easy to get up ‘n running very fast!

Creating a new application, following so-called “best practices” from blogs like these 🙂 and the ‘idiomatic Grails-way’ described in the docs and in tutorials work in the beginning, but there’s always a tipping point — where the application has grown a reasonable size — where one should start following a different, maybe less-Grailsey, strategy.

So what can go wrong by using dynamic finders on a domain class?

Perhaps you can recognize following description of the inception of one of your own early Grails applications?

Day 1: Life is beautiful

You create your core domain classes.

I’ve taken the example domain classes graciously from The Definitive Guide to Grails by Jeff Brown and Graeme Rocher — a classic!

grails create-domain-class Album
grails create-domain-class Song
grails create-domain-class Artist
...

Awesome.

A simple HomepageController is created to show something on your homepage. It should return a collection of, let’s say, albums for a certain artist and consequently a dynamic finder is obviously the simplest way to make this happen.

class HomepageController {
    def list(Artist artist) {
        [albums: Album.findAllByArtist(artist)]
    }
}

You and your team mates marvel at the magic! The productivity is unparalleled. You can query almost anything like this. You’ve made finder friends for life and for the next days the team is burning through the other homepage user stories like there’s no tomorrow.

Fast-forward 14 days.

Day 14: DOA

The homepage comes to a grinding halt everytime any customer opens it. Performance left the building a few days ago. There’s still magic in the air, but it’s working against you: you haven’t got a clue what’s happening.

Surely all the added widgets, dashboard graphs and reporting on the homepage can’t have anything to do with this?

You turn on Hibernate query logging and try to see what queries happen when you re-open the homepage.

The console displays an avalanche of SQL until the homepage finally finishes loading.

Hibernate: select this_.id as id1_2_0_, this_.version as version2_2_0_, this_.name as name3_2_0_ from artist this_ where this_.name=? limit ?
Hibernate: select this_.id as id1_0_0_, this_.version as version2_0_0_, this_.artist_id as artist_i3_0_0_, this_.title as title4_0_0_ from album this_ where this_.artist_id=?
Hibernate: select this_.id as id1_2_0_, this_.version as version2_2_0_, this_.name as name3_2_0_ from artist this_ where (this_.name=?) limit ?
Hibernate: select this_.id as id1_0_0_, this_.version as version2_0_0_, this_.artist_id as artist_i3_0_0_, this_.title as title4_0_0_ from album this_ where (this_.artist_id=?)
Hibernate: select this_.id as id1_2_0_, this_.version as version2_2_0_, this_.name as name3_2_0_ from artist this_ where (this_.name=?)
Hibernate: select this_.id as id1_0_0_, this_.version as version2_0_0_, this_.artist_id as artist_i3_0_0_, this_.title as title4_0_0_ from album this_ where this_.artist_id in (?) limit ?
Hibernate: select this_.id as id1_2_0_, this_.version as version2_2_0_, this_.name as name3_2_0_ from artist this_
Hibernate: select this_.id as id1_0_0_, this_.version as version2_0_0_, this_.artist_id as artist_i3_0_0_, this_.title as title4_0_0_ from album this_ where this_.artist_id=?
etc etc etc

It’s a bunch of similar looking queries, clogging up your console.

Furthermore, on production it’s clogging up your customer’s valuable time, which is now shopping elsewhere on the competitor’s, faster website.

The explanation

Let’s delve into the Grails code, because somewhere something must be causing this.

You remember that simple HomepageController you personally started out with on Day 1, when life was beautiful?

class HomepageController {
    def list(Artist artist) {
        [albums: Album.findAllByArtist(artist)]
    }
}

Well, it’s not there anymore, at least the simple version. For various tables and charts on the homepage, stuff is now split up over multiple classes, such as view builder services and tag libs.

By the logged SQL it looks like every query almost queries the same information, but you can’t tell anymore. Crawling through the code, you can spot around 18 different places where developers copied dynamic finders similar the original one.

At one place:

Artist artist = Artist.findByName(artistName)
Album.findAllByArtist(artist)

(“Ok, not elegant, but it works”)

At another place:

Album.findAllWhere(artist: Artist.findWhere(name: artistName))

(“Sure, seems to do the same, but as a one-liner”)

Again at another place:

def artists = Artist.findAllWhere(name: artistName)
Album.findAllByArtistInList(artists, [max: 1])

(“What, why? And why limiting the results to just 1 here?”)

Again at another place:

Album.where {
    artist == Artist.list().find { a ->
        a.name == artistName
    }
}.findAll()

(“NOOOooo!”)

The pattern

This happened to me a gazillion times before. Me and the team value the simplicitly and power of Grails. Hence, following convention-over-configuration and being idomatic in our Grails development work, we can deliver fast.

The case with the album and artist dynamic finders above is often not far from reality. Even in the same team, different developers eventually will use what I call “locally optimized dynamic finders” everywhere. Why?

  • They’re too easy to start with
  • They’re too easy to keep on using

There’s almost no boilerplate involved in writing what kind of query you need, creating constructs around them or finding collaborators to go through: what you need is as simple as findBy* and what you get is the domain class you’re on.

Dynamic finders are very readable (and easy to unit test) and so team members will

  • copy and adapt them for a slightly different scenario (here we do need sorting, here we only need one instead of the list)
  • try to be smarter by writing the query in a more “clever” way (sometimes resulting in the opposite)
  • not even know of any existing uses in the application and just, by the documentation or by head, try to get the work done from scratch (so they think)

The disadvantages will surface as the application grows:

  • Every query is different in structure, so you can’t tell by the outside whether or not you’ve got actually 18 different queries or just 2. Any query caches (e.g. by Hibernate) are perhaps either less effective or actually not needed — but now just taking up memory.
  • Carefully crafted indices by the DBA created on Day 1 aren’t used 100% anymore at the end by the majority of the queries, because of the arbitrary presence and order of columns in the WHERE-clause. Any database-optimizations become useless, and perhaps slow table scans are introduced instead.
  • Any refactoring to a “find albums for an artist” query, should be carefully hand-crafted at all the 18 different needed places. This causes everyone headaches. And mistakes.

What we can we do about it?

Maybe obvious, but keep it DRY

Centralize things. Just like you do when code gets duplicated for the 2nd or 3rd time, you find a nice place in your application where every developer can find it. You just then need to worry about that one place.

First, the best Grails thing we can do is making such a central place by e.g. creating a regular Grails service such as e.g. an AlbumService.

grails create-service Album
| Created grails-app/services/example/AlbumService.groovy

Secondly, analyse all of your “find albums for an artist” queries and try to converge on just 1 or 2, which should placed into the new service.

Try to find sensible names for some new methods, which in some cases can just be the name of the original dynamic finder such as findAllByArtist:

@Transactional
class AlbumService {

    Album findAllByArtist(Artist artist) {
        Album.findAllByArtist(artist)
    }
}

Third, replace all occurrences of the usage by a call to the new service. Remember our homepage controller? It’ll change like this:

class HomepageController {

    AlbumService albumService

    def list(Artist artist) {
        [albums: albumService.findAllByArtist(artist)]
    }
}

By updating all 18 individual dynamic finders by a call to the newly created service, you’ve given yourself less instead of more maintainance and the freedom to refactor the methods in the new service internally to more performant variants.

By doing this, you might have to change all kinds of unit tests (such as e.g. HomepageControllerSpec, if you have them): where you first used the in-memory store to actually test the dynamic finders on the domain classes, in these tests now you just have to verify the correct service (e.g. AlbumService) is being invoked.

Only the test for AlbumService should deal with testing the queries now.

In the end AlbumService might contain only 1 or 2 resulting queries, which are used from every needed place in the codebase, but are highly optimized for speed, using all kinds of features such as caching, database indices etc. The kind of performance problems which got us in a mess should be a thing of the past. We can now create other problems 🙂

Dynamic finders are great!

If you’re still not sure whether or not I’m advocating to abanondon dynamic finders? No, ofcourse not – dynamic finders are the best thing ever happened since sliced pizza! Start a new project or prototype with them — great! And do the smart thing very soon and, together with the team, _place your queries together__ in an organized place.

Most important lesson I have learned: if all of the queries are in a neat, well-defined place for everyone to see and go to, the most likely it is that team members will default to basic human behaviour — software development is sometimes just psychology — follow the defined path, and color within the nicely defined lines of that part of the codebase. Seeing what’s already there forces people to think of whether or not it’s worth to introduce a completely new query vs altering one of existing ones.

3 thoughts on “Grails Anti-Pattern: Locally Optimized Dynamic Finders Everywhere

  1. I totally agree with you. In my last project we (the team) decided to put all DB-related code in “special” services. They were just Grails Services but we named them xxxRepositoryService. Then we created integration tests for those service and in the rest of the code we just mocked/stubbed that repositoryServices to check behavior or return the data we needed for the test.
    We did that to the extreme, so for example we had methods like:

    Author findById(Long authorId) {
    Author.get(authorId)
    }

    At the beggining we weren’t sure about this but after a couple of months we were very happy with the decission.

    Regards, Iván.

    Like

  2. What are your thoughts on centralizing queries as named queries in the domain objects as opposed to putting them in services?

    Like

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s