HiveBrain v1.2.0
Get Started
← Back to all entries
patternspringMinor

Registration, login and logout in a Grails application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
registrationapplicationloginandlogoutgrails

Problem

I'm completely new to Groovy, but have many years of Java experience, and recently I have started working on a Grails application, currently it contains the following:

  • User and Person domain classes



  • UserService service class



  • UserController controller class to offer a REST API



This application is written using Groovy 2.4.3 and Grails 3.0.2.

I'd like to have this code reviewed and if possible extra attention on the transition from Java to Groovy and on the mixture of Grails and Spring code.

domain/User.groovy

class User {
    static constraints = {
        email email: true, unique: true
        person nullable: true
        registrationDate nullable: true
        loginDate nullable: true
    }

    String email
    String password
    Person person
    Instant registrationDate
    Instant loginDate
}


domain/Person.groovy

class Person {
    static constraints = {
        middleName nullable: true
        city nullable: true
        street nullable: true
        houseNumber nullable: true
        zipCode nullable: true
        email email: true, nullable: true
        bankAccount nullable: true
    }

    static hasMany = [
        telephoneNumbers: String
    ]

    String firstName
    String middleName
    String lastName
    String city
    String street
    String houseNumber
    String zipCode
    String email
    List telephoneNumbers
    String bankAccount
}


conf/spring/resources.groovy

beans = {
    clock(Clock) { bean ->
        bean.factoryMethod = "systemDefaultZone"
    }
}


services/UserService.groovy

```
@Transactional
class UserService {
Clock clock

User createUser(String firstName, String middleName, String lastName, String email, String password) {
if (User.countByEmail(email)) {
throw new CreateUserException("EMAIL_IN_USE")
}

def salt = BCrypt.gensalt(12)
def encodedPassword = BCrypt.hashpw(password, salt)
def user = new User(email: email, password

Solution

Disclaimer: I don't know sqat about Grails

createUser

I have a few concerns in this method.

Using User.countByEmail seems inefficient. I don't know Grails, but the name of this function suggests that it will scan all the users in the database, when it should stop after the first match, effectively doing an EXISTS query.

The string parameter in all the exceptions that you throw look like technical tags, not like user friendly messages, for example:

throw new CreateUserException("EMAIL_IN_USE")


If they are technical tags that they will be used by another process, then to avoid potentially inconsistent uses, it would be better to convert these strings to constants, and refer to them by the constants.

Lastly, this looks like it should be executed in a transaction:

person.save()
    user.person = person
    user.save()


That is, if person is saved but an error occurs while saving user, your backend will be in an inconsistent state.

REST API

Some of the REST endpoints are unconventional:

  • /api/user : this is good for GET requests, to get all users



  • /create_user : the common practice is the same endpoint as the one for getting all users, but with POST only



  • /login_user : not too bad, but the qualifier "user" seems redundant: who else would ever login if not a user?



Unit tests

I look forward for your unit tests, in a next question ;-)

Code Snippets

throw new CreateUserException("EMAIL_IN_USE")
person.save()
    user.person = person
    user.save()

Context

StackExchange Code Review Q#95763, answer score: 6

Revisions (0)

No revisions yet.