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

Reduce amount of calls to database for authentication

Submitted by: @import:stackexchange-codereview··
0
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

  • 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 as UID in 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.

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 response


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(&&) and have one single else statement, like this

if(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 response


I 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 response
if(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 response
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
    }
}

Context

StackExchange Code Review Q#150393, answer score: 3

Revisions (0)

No revisions yet.