platform go challenge rest api #18

artemis13 · 2024-07-10T09:18:09Z

No description provided.

Initialize the program and create the server

create the models and it's structure

read me file so far

replace the local module with remote repository

Set up a GOPROXY enviromental variable and load it from main foe ease of deployment

test custom middleware package for https redirects and jwt auth

The following have been added middleware.Secure CORS settings limit to the number of requests of a user per minute force https redirect -- not working jwt authentication tests pending to be updated....

adding pagination

ApoPsallas

Hello @artemis13. Thank you for your time.
I have some questions for you regarding the implementation. I would love for you to address them.

ApoPsallas · 2024-07-15T13:51:40Z

models/models.go

+type Asset struct {
+	ID          string
+	Type        AssetType
+	Description string
+	Chart       *Chart
+	Insight     *Insight
+	Audience    *Audience
+}


What are some problems that may occur with this model structure? How would you solve them?

I left this question last, because I must admin it seemed as the most difficult to answer. I did a lot of searching and reading regarding structs and whether I shuld have used values or pointers and to be really honest, I haven’t decided yet… So my answer is that taking into consideration that the values of Chart, Insight and Audience could not be change, (the user only changes the description), the use of pointers here is useless.

ApoPsallas · 2024-07-15T13:58:51Z

storage/storage.go

+)
+
+// ConvertStringToUint safely converts a string to uint
+func ConvertStringToUint(s string) (uint, error) {


Is this the correct file to have this helper function?

The ConvertStringToUint is placed in the storage.go file and is being called from the handlers.go file. For the purpose of the project it would be more appropriate if it was placed in the handlers package in a helpers.go file containing various helper functions. As the project grows a utils package should be created containing all the helper functions.

ApoPsallas · 2024-07-15T14:04:57Z

handlers/handlers.go

+	for i, asset := range user.Favorites {
+		if asset.ID == assetID {
+			user.Favorites = append(user.Favorites[:i], user.Favorites[i+1:]...)
+			return c.NoContent(http.StatusNoContent)
+		}
+	}


What would you change to make this more optimized?

In this part of the code I have used re-slicing technique in order to return a new slice which would not contain the user’s Favorite asset that must be deleted. After further reading as you point this issue, I realize that append can be costly both in time and space, and taking under consideration that a user can have an unlimited number of favorites this solution may not be the best. Alternatively, with the specific requirements, a linked list or a hashMap are better solutions over a slice structure for the Favorites in User Struct in order to be faster and with more efficient complexity. Between the two, a hashMap would be preferred especially if we don’t need to maintain an order of the favorites Assets.
In order to optimize the initial approach with the slice structure a remove function could be implemented which removes an element by swapping it with the last element and then slicing off the last element (the order here is not also not maintained as in the hashMap solution), but if I reimplemented the code I would have choose another structure to begin with, and most likely a hashMap.

`func remove( s []Asset, i int) []Asset {
s[i] = s[len(s)-1]
return s[:len(s)-1]
}

for i, asset := range user.Favorites {
if asset.ID == assetID {
user.Favorites = remove(user.Favorites, i)
return c.NoContent(http.StatusNoContent)
}
}
`

ApoPsallas · 2024-07-15T14:10:02Z

handlers/handlers.go

+	for i, asset := range user.Favorites {
+		if asset.ID == assetID {
+			user.Favorites = append(user.Favorites[:i], user.Favorites[i+1:]...)
+			return c.NoContent(http.StatusNoContent)
+		}
+	}


In your opinion could this logic be moved to the storage package?

Yes, this logic could be moved to the RemoveUserFavorite function In storage. In this way the handlers code is cleaner and the storage package keeps the data manipulation logic encapsulated with the storage layer. The code is more well organized this way

ApoPsallas · 2024-07-15T14:18:02Z

storage/storage.go

+var (
+	users = make(map[uint]*models.User)
+	mu    sync.RWMutex
+)


Would there be a benefit to creating a struct to encapsulate these and use that struct as a receiver for the following functions?

Yes, by creating a struct to encapsulate the users map and the mu mutex we can make use of better software design and development practices. We ensure better code management and future code expansions are easier to manage. Also we can control better unauthorizes access to the users map. Also we can centralizing the synchronization logic within the struct and avoid race conditions and data corruptions.

ApoPsallas · 2024-07-15T14:35:52Z

storage/storage.go

+}
+
+// GetUser safely retrieves a user from the map
+func GetUser(userID uint) (*models.User, bool) {


What would happen if the returned model was mutated by the caller outside this function? Is this a safe way to return this struct?

I think I’ve made a big mistake here. The way that it is implemented the caller can affect the original data. Also race conditions are not being avoided here. We should avoid returning a pointer to the user struct here and instead return a copy of the user.

artemis13 · 2024-07-17T07:07:03Z

Hello @artemis13. Thank you for your time. I have some questions for you regarding the implementation. I would love for you to address them.

Hello @ApoPsallas! Thank you for giving me the chance to complete the GWI go platform challenge and for your time reviewing my code. I will look into the questions and the issues that may arise.

artemis13 · 2024-07-18T06:59:07Z

@ApoPsallas I tried to answer your questions the best I could. Thanks for pointing me this issues and please feel free to ask me any other question or clarification. Thanks again for the time you devoted reviewing my implementation.

NikosMas

Hi Artemis 👋
Thank you so much for your effort to complete the assignment and your time to write the documentation!
I have a few questions for you, take your time and post your answers whenever you're ready.

NikosMas · 2024-07-18T14:07:10Z

middleware/auth.go

+
+const testToken = "gwi-token-12345" //for future implementation this token should be encrypted and handled properly
+
+func AuthMiddleware(next echo.HandlerFunc) echo.HandlerFunc {


I see that you chose to use a test token here, but can you describe to me in simple words the steps of proper token handling??

Of course! For the sake of simplicity and time management I have used a simple token-based authentication with a static token but in a real world application a JWT implementation is a more proper approach as it is safer and more reliable. So in order to implement a proper token handling we can use Golang-JWT package and we should go through the following steps:

First of all we have to create the JWT Tokens. We also have to specify a signing method and any relevant info such as username and token expiration date.

Then we have to sign the token with a secret key and return the generated token as a string

Before granting access to protected resources we need to verify it’s authenticity. We provide a callback function to retrieve the secret key used for signing the token. If the token is valid, we continue processing the request; otherwise, we return an error indicating that the token is invalid

To secure protected routes, we need to introduce a login system where users can authenticate and obtain a JWT token

After implementing the login system and obtained a JWT token, we need to modify the protected route to require a valid JWT token for access. In order to do this we can check for a valid JWT token in the Authorization header of the request. If the token is missing or invalid, we return an appropriate error response. Otherwise, we proceed with serving the protected resource.

NikosMas · 2024-07-18T14:10:22Z

routes/routes.go

+	e.GET("/public", handlers.PublicHandler)
+
+	// Group routes that require authentication
+	authGroup := e.Group("/users")


Supposing that you have a complete authorization process (proper token handling, user login & register processes, etc. ), can you think of any changes/optimizations that you could apply in your route paths?

Supposing that we have a complete authorization process the user Id will be a part of the authorization token so this won’t have to be passed in the route as it could be extracted from within the auth functions. After removing the user id from the routes we can further use a nested group for all the routes related to user favorites. So the RegisterRoutes should look like this:

func RegisterRoutes(e *echo.Echo) { // Public routes (no authentication required) e.GET("/public", handlers.PublicHandler) // Group routes that require authentication authGroup := e.Group("/users") authGroup.Use(myMiddleware.AuthMiddleware) // Nested group for user-specific routes userFavoritesGroup := authGroup.Group("/favorites") // Protected routes (require authentication) userFavoritesGroup.GET("", handlers.GetUserFavorites) userFavoritesGroup.POST("", handlers.AddUserFavorite) userFavoritesGroup.PUT("/:asset_id", handlers.EditUserFavorite) userFavoritesGroup.DELETE("/:asset_id", handlers.RemoveUserFavorite) }

All the handlers functions should be updated to so they can retrieve the user id from the context rather than the url param.

NikosMas · 2024-07-18T14:23:57Z

storage/storage.go

@@ -0,0 +1,71 @@
+package storage


If I told you to use a Database system instead of keeping all data in memory

Which kind of database would you choose and why?

Can you give a brief schema of this database (tables definition, relationships, or whatever you would do)

Would you use any ORM to connect and exchange data with the database? Do you have any in mind?

Even thought I am most familiarized with relational databases and I would be more confident in implementing such a solution, I don’t think that this kind of db is suitable for the application. Instead a no-SQL solution like MongoDB should be preferred. Or maybe in a real world application an AWS-based data structure could be used. Unfortunately I have no professional experience with no-SQL solutions but I will try to implement one.
So taking these in mind an implementation for models is shown below:

package models import ( "go.mongodb.org/mongo-driver/bson/primitive" ) type AssetType int type Chart struct { Title string `bson:"title" json:"title"` XAxis string `bson:"x_axis" json:"x_axis"` YAxis string `bson:"y_axis" json:"y_axis"` Data map[string]float64 `bson:"data" json:"data"` } type Insight struct { Text string `bson:"text" json:"text"` } type Audience struct { Gender string `bson:"gender" json:"gender"` BirthCountry string `bson:"birth_country" json:"birth_country"` AgeGroup string `bson:"age_group" json:"age_group"` HoursOnSocialMedia int `bson:"hours_on_social_media" json:"hours_on_social_media"` PurchasesLastMonth int `bson:"purchases_last_month" json:"purchases_last_month"` } type Asset struct { ID primitive.ObjectID `bson:"_id,omitempty" json:"id,omitempty"` Type AssetType `bson:"type" json:"type"` Description string `bson:"description" json:"description"` Chart *Chart `bson:"chart,omitempty" json:"chart,omitempty"` Insight *Insight `bson:"insight,omitempty" json:"insight,omitempty"` Audience *Audience `bson:"audience,omitempty" json:"audience,omitempty"` } type User struct { ID primitive.ObjectID `bson:"_id,omitempty" json:"id,omitempty"` Favorites []Asset `bson:"favorites" json:"favorites"` }

After the models are defined indexes should be created to improve performance. For example indexes could be created on the favorites type field and the favorites id field. Also we could implement some validation logic making sure that values like User.Id and Asset.Id are not empty before performing any operation.
In the application I have used the echo framework so as for the use of an ORM to interact with a MongoDB, the MongoDB Go Driver could be a suitable solution providing all the necessary features to manage complex data structures

NikosMas · 2024-07-18T14:32:46Z

handlers/handlers.go

+	"github.com/labstack/echo/v4"
+)
+
+func GetUserFavorites(c echo.Context) error {


Nice of you to use pagination in this endpoint 👍
I have a question though. If I told you to use a parameter to order the results, in addition to adding the "orderBy" parameter to query params, what would you also change in your current implementation?

In order to make this function even more efficient we can implement sorting options. For sorting options we need an “orderBy” and an “order” param to specify the user’s preference’s. The “orderBy” param contains info about the fields by which the results would be sorted while the “order” param contains info about the type of ordering (asc, desc). Both parameters should be able to accept comma-seperated list of field in order to accept multiple params. For example we may want to retrieve the user’s favorite by id with ascending order and by type with descending order.

// Get orderBy parameter orderBy := c.QueryParam("orderBy") if orderBy == "" { orderBy = "default" // Set a default ordering criteria if not provided } order := c.QueryParam("order") if order == "" { order = "asc" // Default to ascending order } // Parse the orderBy and order fields orderByFields := strings.Split(orderBy, ",") orderFields := strings.Split(order, ",") // Sort the favorites based on the orderBy and order parameter sort.SliceStable(favorites, func(i, j int) bool { for idx, field := range orderByFields { // Determine the direction for this field direction := "asc" if idx < len(orderFields) { direction = orderFields[idx] } switch field { case "id": if favorites[i].ID != favorites[j].ID { if direction == "desc" { return favorites[i].ID > favorites[j].ID } return favorites[i].ID < favorites[j].ID } case "type": if favorites[i].Type != favorites[j].Type { if direction == "desc" { return favorites[i].Type > favorites[j].Type } return favorites[i].Type < favorites[j].Type } // Add more cases as needed for additional fields } } return false })

curl -X GET "http://localhost:1323/users/1/favorites?page=1&limit=10&orderBy=id,type&order=asc,desc" -H "Authorization: gwi-token-12345" -H "Content-Type: application/json"

At this point. if we wanted to go a step further we can also implement filtering options in order to narrow down the list of results before sorting them.
GetUserFavorites_sorting.txt

NikosMas · 2024-07-18T14:39:38Z

Dockerfile.dev

@@ -0,0 +1,18 @@
+# Latest golang image on apline linux
+FROM golang:1.17-alpine


Just a note for here, since you use go 1.22.4 in your go.mod file, you should have a similar version in your docker file.

Yes, you are right… my bad I missed that spot.

artemis13 · 2024-07-21T16:56:40Z

Hi Artemis 👋 Thank you so much for your effort to complete the assignment and your time to write the documentation! I have a few questions for you, take your time and post your answers whenever you're ready.

Hello @NikosMas thank you for taking the time to review my implementation and for the feedback you provided.

NikosMas · 2024-07-23T09:21:00Z

Hi @artemis13, thank you for your responses, I am fully covered!
I have a last question about the deployment of your application, can you describe step-by-step in simple words a deployment process that you would apply (starting from code and ending to be able to play with the application from everywhere)

artemis13 · 2024-07-23T21:25:49Z

Hi @artemis13, thank you for your responses, I am fully covered! I have a last question about the deployment of your application, can you describe step-by-step in simple words a deployment process that you would apply (starting from code and ending to be able to play with the application from everywhere)

Sure!

First of all I would plan the work that should be done and divide it to smaller issues and tasks and I would use a tool like Github Projects to manage tasks, bugs and other issues
I would initialize a Git repository
Then I would set up Github Actions for CI/CD and configure it to automate build, tests and deployment process
I would set up the database by selecting an appropriate database management system (DBMS), configuring the database schema, and ensuring it is accessible to the application.
I would start developing the application, step by step based on the initial plan, I would test it locally and push it to the git repository frequently (based on the issues that were decided initially)
I would write and run tests for the application, including test for database interaction and push changes to repo only if tests pass locally
Based on the issues that arise during development we may have to make changes to the initial plan.
After the completion of the development process a docker file should be created and a docker image should be builded and tested locally. If it works properly it should be commited to the repo
Push the docker image to a container registry
The container image should then be deployed to a development server or a cloud service.
After thorough testing on dev it can be deployed to production

artemis13 added 18 commits July 7, 2024 17:00

initialization

adc5ffd

Initialize the program and create the server

Models

6a81a6e

create the models and it's structure

Handlers and routes

d1b7182

Tests

4e43def

read me

8e82af1

read me file so far

replacing the local module

e18edd2

replace the local module with remote repository

GOPROXY variable

2c3808e

Set up a GOPROXY enviromental variable and load it from main foe ease of deployment

custom midlleware

5b39feb

test custom middleware package for https redirects and jwt auth

Added security

2ddbc7b

The following have been added middleware.Secure CORS settings limit to the number of requests of a user per minute force https redirect -- not working jwt authentication tests pending to be updated....

Pagination

03f01a7

adding pagination

comments

aa3e01b

Updated read me file

19b0010

update read me

91df2a7

update reade me

01ccefc

Docker file

2f10c04

Update README.md

f08ee55

Update README.md

0182537

Update README.md

9f53c27

ApoPsallas reviewed Jul 15, 2024

View reviewed changes

NikosMas reviewed Jul 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

platform go challenge rest api #18

platform go challenge rest api #18

artemis13 commented Jul 10, 2024

ApoPsallas left a comment

ApoPsallas Jul 15, 2024

artemis13 Jul 18, 2024

ApoPsallas Jul 15, 2024

artemis13 Jul 17, 2024

ApoPsallas Jul 15, 2024

artemis13 Jul 17, 2024

ApoPsallas Jul 15, 2024

artemis13 Jul 17, 2024

ApoPsallas Jul 15, 2024

artemis13 Jul 17, 2024

ApoPsallas Jul 15, 2024

artemis13 Jul 17, 2024

artemis13 commented Jul 17, 2024

artemis13 commented Jul 18, 2024

NikosMas left a comment

NikosMas Jul 18, 2024

artemis13 Jul 21, 2024

NikosMas Jul 18, 2024

artemis13 Jul 21, 2024

NikosMas Jul 18, 2024

artemis13 Jul 21, 2024

NikosMas Jul 18, 2024

artemis13 Jul 21, 2024

NikosMas Jul 18, 2024

artemis13 Jul 21, 2024

artemis13 commented Jul 21, 2024

NikosMas commented Jul 23, 2024

artemis13 commented Jul 23, 2024


		const testToken = "gwi-token-12345" //for future implementation this token should be encrypted and handled properly

		func AuthMiddleware(next echo.HandlerFunc) echo.HandlerFunc {

		@@ -0,0 +1,18 @@
		# Latest golang image on apline linux
		FROM golang:1.17-alpine

platform go challenge rest api #18

Are you sure you want to change the base?

platform go challenge rest api #18

Conversation

artemis13 commented Jul 10, 2024

ApoPsallas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artemis13 commented Jul 17, 2024

artemis13 commented Jul 18, 2024

NikosMas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artemis13 commented Jul 21, 2024

NikosMas commented Jul 23, 2024

artemis13 commented Jul 23, 2024