[CLOUDTRUST-1157] Add user_id in events + allow generic HTTP reponse #15
Conversation
… a generic reponse
fperot74
changed the title
|
|
||
| //store the event in the DB | ||
| _, err := cm.db.Exec(insertEvent, auditTime, origin, realmName, agentUserID, agentUsername, agentRealmName, userID, username, ctEventType, kcEventType, kcOperationType, clientID, additionalInfo) | ||
| _, err := cm.db.Exec(insertEvent, auditTime, origin, checkNull(realmName), checkNull(agentUserID), checkNull(agentUsername), |
There was a problem hiding this comment.
Why do we need to check for empty string ?
There was a problem hiding this comment.
It would be better to have NULL in database rather than empty string. NULL is easier to manage when searching for events
| additionalInfo := m["additional_info"] | ||
| additionalInfo := m[CtEventAdditionalInfo] | ||
| if additionalInfo == "" { | ||
| var addNfo = make(map[string]string) |
There was a problem hiding this comment.
Nfo ? is it a typo for Info ?
There was a problem hiding this comment.
I often write Nfo for information... or Info... yes, I'm a little bit lazy
| //additional_info - all the rest of the information from the event | ||
| additionalInfo := m["additional_info"] | ||
| additionalInfo := m[CtEventAdditionalInfo] | ||
| if additionalInfo == "" { |
There was a problem hiding this comment.
We do it only if additionalInfo is empty but I think we should do it anyway.
Initially Sonia did it somewhere else, is it moved here ? or done twice ?
There was a problem hiding this comment.
Done by Sonia in the bridge... This way, it remains compatible with her version and new developments but you have to choose to set additional_info by your self OR let this function set it
http/handlers.go
Outdated
| StatusCode int | ||
| Headers map[string]string | ||
| MimeContent *MimeContent | ||
| ExportToJSON interface{} |
There was a problem hiding this comment.
Could we a type more restrictive than interface{} here ? It seems error prone and it would help to understand what it does
There was a problem hiding this comment.
As discussed together, renamed to JSONableResponse
| if err == nil { | ||
| w.Write(json) | ||
| switch e := rep.(type) { | ||
| case GenericResponse: |
There was a problem hiding this comment.
GenericResponse is HTTP oriented.
The intial purpose was to split it between an answer that can be translated into a specific kind of response, HTTP or gRPC, ... I know that we don't have it anymore but I think it was a good split of concerns.
…nd with a generic reponse
fperot74
changed the title
fperot74
changed the title
Generic response let REST methods be able to choose a status code, set headers, return mime content, ...