patternspringMinor
Registration, login and logout in a Grails application
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:
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
domain/Person.groovy
conf/spring/resources.groovy
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
- 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
I have a few concerns in this method.
Using
The string parameter in all the exceptions that you throw look like technical tags, not like user friendly messages, for example:
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:
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:
Unit tests
I look forward for your unit tests, in a next question ;-)
createUserI 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.