patternkotlinMinor
Reduce amount of calls to database for authentication
Viewed 0 times
callsamountauthenticationdatabasereducefor
Problem
I'm currently having a bit of code, which I just know can be improved a lot. I am just blind to it. All of my code looks quite neat to me, except these parts... The login + authentication process is taken just way too long because too many calls are being made to the database.
Code is written in Kotlin but syntax is almost the same as Java. (Kotlin is a JVM language, capable of running Java).
Some explanation: I have 3 accounts
edit
There are 2 tables, Participant and Employee. They both have a column called
This
Both the participant and the employee can register for an event and need to be authenticated accordingly.
Only an Employee can use the LDAP login functionality, and only a Participant can use the email account or social media account.
Which can all register for an event.
The relevant
```
/**
*@returns UID or empty response --> if no account | registers the participant no matter what
*/
@RequestMapping(value="/events/{eventId}/participants", method=arrayOf(RequestMethod.POST))
fun registerParticipant(@PathVariable eventId:
Code is written in Kotlin but syntax is almost the same as Java. (Kotlin is a JVM language, capable of running Java).
Some explanation: I have 3 accounts
- Plain email accounts - will get a random UUID on succesfull authentication --> used as API key
- Social media accounts - On first call to API sends an
accessToken--> returns UID from Firebase --> used as API key. The API key is stored asUIDin the database and is returned to the user. This UID will be validated on each request.
- LDAP accounts - could send a device_id (if logged in from mobile device) --> generates random UUID --> used as API key. The random UUID gets stored in the database as UID and will be validated on each request
edit
There are 2 tables, Participant and Employee. They both have a column called
uid or api_key which is quite inconsistent but they're the exact same.This
uid or api_key will either be random generated (email or LDAP) or will be retrieved from Firebase by validating the accessToken.Both the participant and the employee can register for an event and need to be authenticated accordingly.
Only an Employee can use the LDAP login functionality, and only a Participant can use the email account or social media account.
UID or 'api_key' will be returned to the mobile device/browser and will be authenticated by the REST API.Which can all register for an event.
The relevant
Spring REST endpoints which bind the request to a Service' method:```
/**
*@returns UID or empty response --> if no account | registers the participant no matter what
*/
@RequestMapping(value="/events/{eventId}/participants", method=arrayOf(RequestMethod.POST))
fun registerParticipant(@PathVariable eventId:
Solution
I noticed something with this if statement that bothered me.
you are placing another if statement directly inside of an else statement and increasing your indentation more than necessary, you should be using an else if instead, like this:
I noticed another if statement that looks overly complex
your two outer if statements have an else statement that is exactly the same, this is very redundant and they can be merged together to prevent another level of indentation and repeating yourself.
I would merge the if statements with an And(
I found another instance of the first thing I mentioned
in this instance you can also minimize the indentation and use an else if instead of an if inside of the else.
if(uid != null){
return ServiceProvider.participantService.registerParticipant(eventId, registeredWith, uid)
}else{
if(accessToken != null){
return ServiceProvider.participantService.registerWithAccessToken(participant, accessToken, eventId, registeredWith)
} else {
return ServiceProvider.participantService.registerWithoutUID(participant, eventId, registeredWith)
}
}you are placing another if statement directly inside of an else statement and increasing your indentation more than necessary, you should be using an else if instead, like this:
if (uid != null) {
return ServiceProvider.participantService.registerParticipant(eventId, registeredWith, uid)
} else if(accessToken != null) {
return ServiceProvider.participantService.registerWithAccessToken(participant, accessToken, eventId, registeredWith)
} else {
return ServiceProvider.participantService.registerWithoutUID(participant, eventId, registeredWith)
}I noticed another if statement that looks overly complex
if(participant is Participant){
if(participant.email != ""){
response.data = participant
// if user already exists -> only register, else add and register
val id: Int = participantExists(participant)
if(id > 0) {
if(participantRegistered(participant, eventId)) {
response.message = alreadyRegisteredMessage
} else {
updateUID(participant, uid, registeredWith, eventId)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
} else {
// participant doesn't exist --> insert including new UID from the token
addParticipant(participant, uid)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
} else {
response.error = errorMessage
}
}else {
response.error = errorMessage
}
return responseyour two outer if statements have an else statement that is exactly the same, this is very redundant and they can be merged together to prevent another level of indentation and repeating yourself.
I would merge the if statements with an And(
&&) and have one single else statement, like thisif(participant is Participant && participant.email != ""){
response.data = participant
// if user already exists -> only register, else add and register
val id: Int = participantExists(participant)
if(id > 0) {
if(participantRegistered(participant, eventId)) {
response.message = alreadyRegisteredMessage
} else {
updateUID(participant, uid, registeredWith, eventId)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
} else {
// participant doesn't exist --> insert including new UID from the token
addParticipant(participant, uid)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
}else {
response.error = errorMessage
}
return responseI found another instance of the first thing I mentioned
if(participantRegistered(participant, eventId)){
response.message = "U staat al ingeschreven voor dit evenement!"
return response
}else {
// boolean to check if insertion went well
if(registerForEvent(participant, eventId, registeredWith)){
// update data in case participant moved/changed phone/wrongly entered previously
updateParticipant(participant)
// TODO RETURN UID - if exists
response.message = "U heeft zich succesvol ingeschreven voor dit evenement."
return response
}
else {
// throw 400
response.message = "Er is iets misgegaan, probeer het later opnieuw"
return response
}
}in this instance you can also minimize the indentation and use an else if instead of an if inside of the else.
if (participantRegistered(participant, eventId)) {
response.message = "U staat al ingeschreven voor dit evenement!"
return response
} else if(registerForEvent(participant, eventId, registeredWith)) {// boolean to check if insertion went well
// update data in case participant moved/changed phone/wrongly entered previously
updateParticipant(participant)
// TODO RETURN UID - if exists
response.message = "U heeft zich succesvol ingeschreven voor dit evenement."
return response
} else {
// throw 400
response.message = "Er is iets misgegaan, probeer het later opnieuw"
return response
}Code Snippets
if(uid != null){
return ServiceProvider.participantService.registerParticipant(eventId, registeredWith, uid)
}else{
if(accessToken != null){
return ServiceProvider.participantService.registerWithAccessToken(participant, accessToken, eventId, registeredWith)
} else {
return ServiceProvider.participantService.registerWithoutUID(participant, eventId, registeredWith)
}
}if (uid != null) {
return ServiceProvider.participantService.registerParticipant(eventId, registeredWith, uid)
} else if(accessToken != null) {
return ServiceProvider.participantService.registerWithAccessToken(participant, accessToken, eventId, registeredWith)
} else {
return ServiceProvider.participantService.registerWithoutUID(participant, eventId, registeredWith)
}if(participant is Participant){
if(participant.email != ""){
response.data = participant
// if user already exists -> only register, else add and register
val id: Int = participantExists(participant)
if(id > 0) {
if(participantRegistered(participant, eventId)) {
response.message = alreadyRegisteredMessage
} else {
updateUID(participant, uid, registeredWith, eventId)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
} else {
// participant doesn't exist --> insert including new UID from the token
addParticipant(participant, uid)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
} else {
response.error = errorMessage
}
}else {
response.error = errorMessage
}
return responseif(participant is Participant && participant.email != ""){
response.data = participant
// if user already exists -> only register, else add and register
val id: Int = participantExists(participant)
if(id > 0) {
if(participantRegistered(participant, eventId)) {
response.message = alreadyRegisteredMessage
} else {
updateUID(participant, uid, registeredWith, eventId)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
} else {
// participant doesn't exist --> insert including new UID from the token
addParticipant(participant, uid)
registerForEvent(participant, eventId, registeredWith)
response.message = registerSuccessMessage
}
}else {
response.error = errorMessage
}
return responseif(participantRegistered(participant, eventId)){
response.message = "U staat al ingeschreven voor dit evenement!"
return response
}else {
// boolean to check if insertion went well
if(registerForEvent(participant, eventId, registeredWith)){
// update data in case participant moved/changed phone/wrongly entered previously
updateParticipant(participant)
// TODO RETURN UID - if exists
response.message = "U heeft zich succesvol ingeschreven voor dit evenement."
return response
}
else {
// throw 400
response.message = "Er is iets misgegaan, probeer het later opnieuw"
return response
}
}Context
StackExchange Code Review Q#150393, answer score: 3
Revisions (0)
No revisions yet.