Wednesday, 27 August 2008

A Productivity Hint Explained

In an earlier post, I mentioned that writing code that looks like this:

myObject."$localVariable".attribute = 42

... is a bad idea for three reasons. 

Here's the three reasons I was looking for:

1 Violating encapsulation

You don't want to manipulate the guts of other objects directly. If we made a habit of just reaching into the objects near the current chunk of code and doing whatever we want, we'd soon have a codebase which is incredibly difficult to change because whatever piece of code you're looking at now could be potentially affected by another piece of code writing over local state. 

Unit tests work by exercising the public interface of the object i.e. those services that are made available to the community of other collaborating objects in the system. If any old thing can reach in and mess around with the internal state then there wouldn't be any point in having unit tests because you can't rely on the assumptions on which those tests are based.

Every programmer learning his or her first object language used to be taught encapsulation, but I'm still amazed how many really experienced people still don't seem to take this seriously. It's not an aesthetic thing. If you don't get this please give up.

Incidentally the  Sharble and Cohen study from Boeing provides some evidence that designs that use lots of getters and setters to centralise design in a few classes also makes the code slower because you need to execute more instructions just to (needlessly) push all the data around.

2 It makes the object graph very fragile

This is about the Law of Demeter. Basically when you write the expression myObject.x.y.z.doWhatever() you're assuming that there's a whole bunch of things stuck together, at the end of which is an object that can doWhatever(). 

The code "myObject.x.y.z.doWhatever()" makes a lot of needless assumptions:

  • myObject knows x
  • x knows y
  • y knows z

... and all it really needs is to be given z as a parameter.

If any of the relationships between anObject, x, y, and z changes  we get an entirely preventable  breakage.

Some people might find it difficult to restructure this code so that you get access to z without rummaging through an object graph (what some people still seem to call a "data structure".)  See what I've got to say  below about responsibility-driven design to help with this.

3 It's a gratuitous abuse of metaprogramming.
In other words, it's slower and far more complicated that it should be. Incidentally, the business with ."$whatever" is hiding $whatever from compiler  because it needs to be evaluated at runtime rather than compile time. It also makes things an order of magnitude more difficult for a refactoring IDE. I doubt that there's an easy way for the IDE to refactor things automatically when you rename an attribute or to even notice that something's wrong when you move the attribute to another class.

anObject."$whatever" = 42

The metaprogramming here with ."$whatever" only became necessary because we're trying to poke away at the guts of another object - something we shouldn't be attempting to do in the first place. We should get anObject to do something, rather than just treat it as a passive store of data (what some people used to more honestly describe as a "record".)



At least those are the three most pressing things that spring to mind.

A funnier treatment of points 1 and 2 can be found in The Tragic Tale of POTS and his friend SPOT. Grails developers who've read about POTS will appreciate the irony of having a DogWalkingService.


How to avoid writing this kind of thing / how to fix it

The best way to avoid writing code like this is to think in terms of "responsibility-driven design" - that's the fine art of putting the data right next to the code that needs it - in the same class. 

A long time ago, people doing extreme programming knew about this. In fact, RDD was invented by the guys who invented extreme programming. Shame it's not so widely used or publicised.

Doing CRC sessions is a very agile-friendly way to do RDD - and was done on the first ever XP project C3.


A lot of this stuff seems obvious (at least points 1 and 2). It's surprising how many people don't seem to get this. 

Extreme programming is founded on the concept of lowering the cost of change. The points I've made here aren't theoretical or style-related. This is vital. This is about the basic stuff that makes everything else we do possible.

Deploy extra WARs with your Grails App

Should you find yourself needing to deploy other web-app WARs with your Grails app, when you want the Grails app under development to run at the root context, this is for you.

This applies specifically to running the app on command line (grails run-app), as compared to packaged WARs; you would not need to do this in production for Jetty since Jetty applies the logic below already - we needed this to automate tests which spanned our app and its integration with another.

First off, if you haven't already deployed your Grails app to the root context, you should refer to this.

Now that you are running your grails app at the root context, you will need to find a suitable location (MY_OTHER_WARS below) to place any additional WARs so that the Grails startup can pick up on it.

This location is referenced in the $YOUR_PROJECT_HOME/scripts/Events.groovy where you will need to add the following

eventConfigureJetty = { Server server ->

   def grailsApp = server.handler
   server.removeHandler(grailsApp)

   def handlers = []
   handlers << grailsApp
   ContextHandlerCollection contextHandlerCollection = new ContextHandlerCollection()

   warProjects.each {
      WebAppContext otherWarWebContext = new WebAppContext("${basedir}/MY_OTHER_WARS/${it}.war", "/${it}")
      handlers << otherWarWebContext
   }

   handlers.each {
      contextHandlerCollection.addHandler(it)
   }

   server.setHandlers(contextHandlerCollection)
}

This ensures that when you start your application on the command line, the default mode of adding the grails app to the Jetty server is overridden with a Jetty ContextHandlerCollection.

The problem with not doing so and just adding more Handlers to the Jetty server means that Grails intercepts all contexts/URLs on the app, and will fail to pass the call for a context onto the appropriate other WAR.

Connection Pooling

I've noticed there isn't that much information about how to setup connection pooling with grails on the mailing lists so I figured I'd put up a quick post about it. In this example I want to create c3p0 connection pools for all profiles that use postgreSQL.In resources.groovy we add the following:
import com.mchange.v2.c3p0.ComboPooledDataSource
import org.codehaus.groovy.grails.commons.ConfigurationHolder

def config = ConfigurationHolder.config

if(config.dataSource.url =~ 'postgresql') {
dataSource(ComboPooledDataSource) {
driverClass = 'org.postgresql.Driver'
user = config.dataSource.username
password = config.dataSource.password
jdbcUrl = config.dataSource.url
maxStatements = 180
minPoolSize = 10
acquireIncrement = 5
maxPoolSize = 100
}
}
and something like this to your configuration (Config.groovy or wherever you keep your profile configurations):
dataSource {
username = 'postgres'
password = ''
url = 'jdbc:postgresql://127.0.0.1:5432/databasename'
}
Fire it up with the right profile and you should see the 10 initial connections to your postgres database by running
ps -U postgres u at the console.

Can haz cuneckshun puling!

Tuesday, 26 August 2008

Using Liquibase DropAll Automagically

If you're using the grails liquibase plugin you might find it useful to have it run on application startup (e.g. when the war file is deployed) rather than using the grails command line target (i.e. grails migrate). One way to achieve this is to add an entry for the liquibase spring bean in your resources.groovy like so:

liquibase(SpringLiquibase) {
dataSource = dataSource
changeLog = "classpath:migrations/changelog.xml"
executeEnabled = true
}

For certain environments (continuous build etc.) it may also be useful to have liquibase clear out the database and build the schema from scratch. The main Liquibase class has a dropAll() method that looks like it'll do the trick (there's also a 'grails drop-all' command), so let's create a spring bean that'll run before the liquibase migration kicks in (it'd be a bit pointless to blow away the database after you've just created it). The spring
'depends-on' attribute should work nicely here but it would appear that grails' BeanBuilder does not support attributes (foiled again). We can, however, implement BeanFactoryPostProcessor to achieve the desired effect:

import java.sql.Connection
import liquibase.spring.SpringLiquibase
import org.springframework.beans.factory.config.BeanDefinition
import org.springframework.beans.factory.config.BeanFactoryPostProcessor
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory

class LiquibaseDropAll extends SpringLiquibase implements BeanFactoryPostProcessor {

String runBefore = 'liquibase'

void afterPropertiesSet() {
Connection conn = null
conn = getDataSource().getConnection()
super.createLiquibase(conn).dropAll()
}

//make liquibase bean depend upon this bean so that dropAll runs before migration
void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) {
BeanDefinition beanDefinition = beanFactory.getBeanDefinition(runBefore)
beanDefinition.setAttribute('depends-on', beanName)
}

}
The last thing to do is add this bean to your resources.groovy, most probably with a conditional based on environment:

def config = org.codehaus.groovy.grails.commons.ConfigurationHolder.config
if(config.dataSource.liquibase.dropAll) {
liquibaseDropAll(LiquibaseDropAll) {
dataSource = dataSource
}
}

Now just add something like:

dataSource {
...
liquibase.dropAll = true
}

to the environments you want to liquify and you're away.

Monday, 25 August 2008

Lock, Stock and Two Fuming Developers

Oh Hai!

If you have the misfortune of having to deal with locks in your postgreSQL database, run the following to find out if there are any nasty ExclusiveLocks (whole table locks) on the loose:

select * from pg_locks where mode = 'ExclusiveLock';

There are 4 types of lock in postgreSQL, listed here in order of decreasing niceness:
  • AccessShareLock
  • RowShareLock
  • RowExclusiveLock
  • ExclusiveLock
If you're running some tests that create your lock condition it's sometimes useful to quickly look at what's happening on the console:

watch -n1 ps -U postgres u

Lastly, make sure your postgreSQL version doesn't have REPLICA printed on the side :-/

Tuesday, 19 August 2008

When Groovy Property Access Attacks!

A quick best-practice hint...

Groovy's property access generally leads to terser code, however there's (at least) one time when using it is not a good idea; when you want to know what class an object is.

println x.class.name may work for most types of object, but what happens if x is, hmm let's say, a Map?

You probably won't be surprised to find the following code fails.

    def x = [:]
assert x.class == java.util.LinkedHashMap

Even more entertainingly, this code will throw NullPointerException

    def x = [:]
assert x.class.name == 'java.util.LinkedHashMap'

Tuesday, 12 August 2008

Overriding constraints

I didn't know you could override constraints until the other day. Here is an example of overriding and then testing the constraints with the new testing plugin. Testing constraints with the new plugin is so easy I suggest every team uses it. We started using it for our team. Here's the code:

class Parent {
String name
String body

static constraints = {
name(blank: false, nullable:false)
body(blank: false, nullable:false)
}
}

class Child extends Parent{
static constraints = {
name(blank: true)
body(blank: true)
}
}


------------Tests using the testing plugin ---------------
import grails.test.GrailsUnitTestCase
import grails.test.MockUtils

class ChildTests extends GrailsUnitTestCase{

void testConstraints() {
// Mock the validate() method.
registerMetaClass(Child)
MockUtils.prepareForConstraintsTests(Child)

registerMetaClass(Parent)
MockUtils.prepareForConstraintsTests(Parent)

def testInstance = new Child()
def errors = testInstance.validate()
assertEquals 0, errors.size()

// Test the parent constraints
testInstance = new Parent(name:' ' )
errors = testInstance.validate()
assertEquals 2, errors.size()
assertEquals "blank", errors["name"]
assertEquals "nullable", errors["body"]
}
}

Happy testing :)

Monday, 11 August 2008

Today's Object-Oriented Productivity Hint

Please try using encapsulation before attempting metaprogramming or functional programming.

Exercise:

Find three reasons not to write this kind of code:

myObject."$localVariable".attribute = 42

This kind of code is wrong on so many levels that if Ali G wrote web apps, he'd love it.


Last week's Retrospective

On Wednesday I ran my first retrospective. I decided to focus on some of the positive values that make up a good team: Simplicity, Feedback, Courage, Respect and Communication.

Here is a link to my write up of the retrospective.

Sending emails from Grails apps

We're trying this on for size...

import org.springframework.mail.MailSender
import org.springframework.mail.SimpleMailMessage

class EmailService {

boolean transactional = false
MailSender mailSender
SimpleMailMessage mailMessage // A template message auto-wired from resources.groovy

boolean sendMessage(String to, String subject, String body) {
boolean result = true
try {
SimpleMailMessage message = new SimpleMailMessage(mailMessage)
message.to = [to]
message.subject = subject
message.text = body
mailSender.send(message)
} catch(Exception e) {
log.error("Unable to send message to ${to} with subject ${subject}", e)
result = false
}
return result
}
}

import org.springframework.context.MessageSourceAware
import org.codehaus.groovy.grails.web.pages.GroovyPagesTemplateEngine
import org.springframework.context.MessageSource
import org.springframework.context.i18n.LocaleContextHolder
import org.codehaus.groovy.grails.web.pages.GroovyPageTemplate

class NotificationService implements MessageSourceAware {

EmailService emailService
GroovyPagesTemplateEngine groovyPagesTemplateEngine
MessageSource messageSource

boolean confirmRegistration(Person person) {
String subject = getMessage('email.registration.confirmation.subject', [])
return emailService.sendMessage(person.email, subject, getEmailBody('registrationConfirmation', [person: person]))
}

boolean registrationSuccess(Person person) {
String subject = getMessage('email.registration.successful.subject', [])
return emailService.sendMessage(person.email, subject, getEmailBody('registrationSuccessful', [person: person]))
}

private String getEmailBody(String templateName, Map model) {
return render("/modules/emails/_${templateName}.gsp", model)
}

private String render(String templateUri, Map model) {
// I wonder if templates can be cached
GroovyPageTemplate template = groovyPagesTemplateEngine.createTemplate(templateUri)
Writable w = template.make(model)
StringWriter sw = new StringWriter()
PrintWriter pw = new PrintWriter(sw)
w.writeTo(pw)
return sw.toString()
}

private String getMessage(String code, List args) {
return messageSource.getMessage(code, (Object[]) args, LocaleContextHolder.locale)
}

void setMessageSource(MessageSource messageSource) {
this.messageSource = messageSource
}
}


Few gotcha's though...

1. TagLibs don't work in the GSP templates. This causes a real headache if you need to use <g:message ... /> etc.

2. I'm not sure whether the templates can be cached. It's probably expensive to keep creatating them each time.

Testing

We're using wiser to stub the SMTP server, e.g.

import org.subethamail.wiser.Wiser
import org.subethamail.wiser.WiserMessage

class EmailServiceTests extends GroovyTestCase {

Wiser wiser
EmailService emailService

void setUp() {
wiser.messages.clear()
}

void testThatICanSendAnEmail() {
assertEquals(0, wiser.messages.size())
emailService.sendMessage('roundhouse@beta.xyz.com', 'Cool beans', 'Well done, you managed to register')
assertEquals(1, wiser.messages.size())

WiserMessage message = wiser.messages.iterator().next()
assertEquals('roundhouse@beta.xyz.com', message.envelopeReceiver)
assertEquals('Cool beans', message.mimeMessage.subject)
assertEquals('Well done, you managed to register', message.mimeMessage.content)
}

void testThatEmailServiceReturnsFalseOnFailure() {
emailService.log.logger.setLevel(org.apache.log4j.Level.OFF)
wiser.stop()
assertFalse('Unexpected successful email send', emailService.sendMessage('roundhouse@beta.xyz.com', 'subject', 'message'))
wiser.start()
emailService.log.logger.setLevel(org.apache.log4j.Level.ERROR)
}
}

We also use wiser in our selenium tests by adding the following action to our test fixture controller,


def smtp = {
List messages = wiser.messages.collect { message ->
String to = message.envelopeReceiver
String from = message.envelopeSender
String subject = message.mimeMessage.subject
String content = message.mimeMessage.content
return [to: to, from: from, subject: subject, content: content]
}
Map model = getJsonModel(messages)
render(view:'index', model: model)
}


Config

It's easy to pick whether to use the real smtp server or wiser in resources.groovy

switch(GrailsUtil.environment) {
case 'prod':
mailSender(org.springframework.mail.javamail.JavaMailSenderImpl) {
host = 'mail1.xyz.com'
}
mailMessage(org.springframework.mail.SimpleMailMessage) {
from = 'info@ xyz.com'
}
break
case ['qa', 'beta']:
mailSender(org.springframework.mail.javamail.JavaMailSenderImpl) {
host = 'mail1.xyz.com'
username = 'info@beta.xyz.com'
password = 'shh'
}
mailMessage(org.springframework.mail.SimpleMailMessage) {
from = 'info@beta.xyz.com'
}
break
default:
wiser(org.subethamail.wiser.Wiser) { bean ->
bean.initMethod = 'start'
bean.destroyMethod = 'stop'
bean.lazyInit = true
port = 2500
}
mailSender(org.springframework.mail.javamail.JavaMailSenderImpl) {
host = 'localhost'
port = 2500
}
mailMessage(org.springframework.mail.SimpleMailMessage) {
from = 'info@beta. xyz.com'
}
break
}