Go-challenge implementation #17
Conversation
Wiz Scan Summary
|
|
Hi @arhsxro, Thank you very much for your time and effort. |
| GetUserFavoritesFunc func(ctx context.Context, userID, filterType string, page, pageSize int) ([]models.Asset, error) | ||
| AddFavoriteFunc func(ctx context.Context, userID string, asset models.Asset) error | ||
| RemoveFavoriteFunc func(ctx context.Context, userID, assetId string) error | ||
| UpdateDescriptionFunc func(ctx context.Context, userID, assetID, newDescription string) error |
There was a problem hiding this comment.
Thank you for the feedback!The truth is I spend a lot of time in tests.
| db *sqlx.DB | ||
| } | ||
|
|
||
| func isValidAssetType(paramType string) bool { |
There was a problem hiding this comment.
Nice that you added a validation. Can you think of a better a way that this could be handled?
There was a problem hiding this comment.
I could probably move the validation to my handler in order not to proceed more if there is a validation err.
Also I could use a map instead of slice to avoid the loop.
|
|
||
| // Adds a new favorite asset for a user in the database | ||
| func (store *PostgresStore) AddFavorite(ctx context.Context, userID string, asset models.Asset) error { | ||
| tx, err := store.db.Begin() //begin transaction |
There was a problem hiding this comment.
Could you describe why you chose to use transactions?
There was a problem hiding this comment.
The truth is that I wasn't 100% sure if i should use a transaction since it only adds/delete/edit a single asset to the db.Allthough i thought that maybe i get at the same time 2 requests where the first 1 is to delete an asset and the second one is to edit the same asset.So in this case with the use of transaction they do not interfere with each other, maintaining the integrity of the data.Another scenario could be that 1 request is to get the favorite assets and the 2 request to add an asset.
api/handlers.go
Outdated
| go func() { | ||
| wg.Wait() | ||
| close(errCh) | ||
| }() |
There was a problem hiding this comment.
Can you explain the use of this goroutine?
There was a problem hiding this comment.
Basically in this goroutine i wait for all the goroutines that add an asset to finish in order to close the error channel.By doing so i can then check for any errors in the channel
| http.Error(w, "Request timed out", http.StatusGatewayTimeout) | ||
| } else { | ||
| log.Println("Error on executing the query for asset ID ", assetErr.Asset.ID+" and error message: ", assetErr.Err) | ||
| http.Error(w, assetErr.Err.Error(), http.StatusInternalServerError) |
There was a problem hiding this comment.
The information that this error has, what could it be?
Is it safe to send to the outside world these kind of messages?
There was a problem hiding this comment.
I think that this : assetErr.Err.Error() in the http.Error should not be displayed.It may contains more detailed information about what happened internally.(maybe information for our db or some other sensitive informaiton).
My implementation for the platform-go-challenge.