Skip to content
jmctee edited this page Sep 13, 2010 · 4 revisions

Joe McTee’s comments
Comments on chapter 3, Grails In Action, MEAP version posted 12/31/2008:

========

page 46 – In Figure 1, “follows / following” should be “followers / following”

========

page 49 – Section 3.2.1. Test was created in ./test/unit/UserTests.groovy, not ./test/integration/UserTest.groovy. If I make changes to this test as indicated and run them, the test fails. This makes sense, since the test is being run as a unit test rather than an integration test, so the database is not being started. Manually moving the test to the ./test/integration directory and then re-running the test results in it passing.

Tested with grails 1.0.4 and the test was placed in the correct directory.

Is this a bug in the latest version of grails or a change in behavior? I suspect the former.

========

page 60 – In code for updatePassword,

/user.password = params.password/

should be

/user.password = params.newpassword/

========

page 65 – “From the output of our test case we’ll see that something is awry:”, the default output appears to show up in ./tests/reports/TEST-UserTests-out.txt, not the screen. I didn’t see this mentioned anywhere. Might be confusing for someone not familiar with the the default logging setup. Consider discussing the Grails logging system early on, since it is an important debugging tool.

========

page 65 – “So how about we set about fixing those up with a refreshed test:”, I find the wording of this section a little confusing, since we didn’t really “fix” anything in the updated test, but rather demonstrated both behaviors of the validate() method. The test was green prior to the change.

========

page 68-69 – The code in the validator does not include any of the coolness discussed in the previous sections. Shouldn’t the changes presented be included in the app going forward?

========

page 70 – Since we have only been introduced to domain objects in this chapter, a reminder that the domain object can be created using “grails create-domain-class” is appropriate here, along with a reminder that it will be created in ./grails-app/domain/Profile.groovy.

========

page 72 – The code snippet for User should provide a little more context on where the new line should be added. The idea is to give the reader an idea about what an idiomatic domain object should look like.

========

page 72-73 – More details about where the test should be added are needed. In addition, you will need to provide instructions about modifying the previous test, testEvilSave, which attempts to initialize a field no longer available in the User class, homepage. Otherewise, this test will fail. A note about why the test, testSaveThenDelete, does not fail would also be helpful (it uses a map initializer and items in the map that do not exist in the class are ignored, you did mention this earlier, but reiteration of this might help).

========

page 73 – Since we are adding slowly to the test, consider changing the user query code snippet as follows:

def user2 = User.findByUserId('joe')
user2.posts.each { post -> 
    println "${post.content} created on ${post.created}"
}

Otherwise, adding it to the test fails because the user variable is defined twice. The other option is to remove the def, but this makes it clear that we are getting information from the DB and putting it in a new location.

========

page 75 – You did not explicitly show how the User class needed to be modified.

========

page 76 – I could use some more explanation (maybe a concrete example) around this statement:

But let’s take the trickier situation of Tags. A Post may have many Tags, and each Tag may relates to
more than one Post. In this case, GORM settles things by the belongsTo tag. If there is no belongTo tag
defined on the object, then no cascades will happen at all. So the cascade happens in neither direction and
we’re on our own.

========

page 76 – Section 3.6.2, adding the bootstrap code does not seem to add anything to the database. I added some debug code, as follows:

def init = { servletContext ->
    if (User.count() == 0) { 
      println "Fresh Database. Creating ADMIN user." 
      def user = new User(userId: "admin",
                          password: "password",
                          profile: new Profile(email: "admin@localhost"))
      println "Saving ADMIN user ${user.userId}" 
      user.save(flush:true) // Note that I added the flush as well
    }
    
    println "User count: ${User.count()}"
}

Here is what the run looks like:

Cryptonomicon:hubbub joemctee$ grails run-app
Welcome to Grails 1.1-beta2 - http://grails.org/
Licensed under Apache Standard License 2.0
Grails home is set to: /opt/grails

Base Directory: /Users/joemctee/Projects/grina/hubbub
Running script /opt/grails/scripts/RunApp.groovy
Environment set to development
  [groovyc] Compiling 1 source file to /Users/joemctee/.grails/1.1-beta2/projects/hubbub/classes
Running Grails application..
Hibernate: 
    select
        count(*) as y0_ 
    from
        users this_
Fresh Database. Creating ADMIN user.
Saving ADMIN user admin
Hibernate: 
    select
        count(*) as y0_ 
    from
        users this_
User count: 0
Server running. Browse to http://localhost:8080/hubbub

In addition to the user count being zero, when I navigate to http://localhost:8080/hubbub/user/list, no users are shown. Not sure why the admin user is not being persisted. Note that the only sql logging that occurs are the queries for the user count.

========

page 82 – Section 3.7.5, I am assuming we are meant to use the grails console for this section, based upon a comment in the previous section. “Let’s re-issue that list() call” was a bit confusing, where did we issue the first list command?

If the intent is to use the console, I suggest the following addition to the example:

def users = User.list([sort:'userId', order:'asc', max:5, fetch:[posts:'eager']])
users.each {println "${it.userId}"}

This provides little better output in the console.

========

General – Many of the examples cannot be cut and pasted from the pdf into the console or a file without significant re-editiing. The principle problem is that the text of the examples uses smart quotes, which ar often not the single or double quote expected by the groovy compiler. Not sure if this can be fixed, but it would be nice if it could.

========

General – As you note in the conclusion, there is a lot of info in this chapter, but it is excellent material, presented in an easy to understand order that builds upon what has already been presented. The only thing missing is the “cookbook” instructions a newb will want. More text that explicitly instructs which files to modify and what commands to run would really put the icing on a great chapter. Your approach in this chapter epitomizes an agile approach and by more explicitly describing the steps needed, not only will your readers be learning grails, but also agile (and some TDD) best practices.

End of Joe McTee’s Comments


Grails now creates unit tests by default. Make sure to update the tests in the chapter or tell the user to run “grails create-integration-test”.


Section 3.6.2 uses “admin@localhost” as an e-mail address, but this does not pass e-mail validation. You need at least one “.” in the host, e.g. “[email protected]”.


page 76 – Section 3.6.2, the above comment fixed things. I suggest the following change to the code in the book, since it seems to be best practice and will uncover issues like this faster:

def init = { servletContext ->
    if (User.count() == 0) { 
      println "Fresh Database. Creating ADMIN user." 
      def user = new User(userId: "admin",
                          password: "password",
                          profile: new Profile(email: "[email protected]"))
      println "Saving ADMIN user ${user.userId}" 
        if (user.validate()) { 
         user.save() 
        } else { 
           // Display validation errors and cleanup
           user.errors.each { err -> println err } 
            user.discard() 
            println 'Validate failed'
        }
    }

    println "User count: ${User.count()}" 
}