GWI favourites challenge by emanonk · Pull Request #70 · GlobalWebIndex/platform-go-challenge

emanonk · 2026-02-02T15:11:31Z

No description provided.

…jection based on env

alexandros-georgantas

Thank you for taking the time implementing this assignment!
I have left some comments and I would like to hear your thoughts.

Thanks,
Alex

alexandros-georgantas · 2026-02-03T08:06:42Z

+	Id          string    `json:"id"`
+	Name        string    `json:"name"`
+	Description string    `json:"description"`
+	UserId      string    `json:"userId"`


Could you explain why both asset and favorite have user correlation? Is user id here needed?

for the favorite I need both UserID and AssetID to link an asset with the user. For assets I treat them as a separate service. So the user creates an asset based on their input attributes. We still want to make sure that this asset belongs to one user. So, an asset that belongs to a user does not mean that it will be in their favorites. Also we might have assets that the onwer is one user and has shared permissions to other users to view.

alexandros-georgantas · 2026-02-03T08:07:51Z

+
+import "time"
+
+type Asset struct {


Could you suggest a different type for the Asset entity?

based on my comment about the GetAllAssets, I can change that to
type Asset struct {
ID string
Type AssetType
Name string
Description string
UserID string
CreatedAt time.Time
UpdatedAt time.Time

Insight *InsightData Audience *AudienceData Chart *ChartData

}

so something closer to the DTO but cleaner with the type to indicate what is not null.

alexandros-georgantas · 2026-02-03T08:40:28Z

+)
+
+// AssetRepository defines the minimal interface the application needs from a repository.
+type AssetRepository interface {


what will happen in the case that someone wants all the available assets at once?

For the exercise, I assumed each AssetType could come from a different source/service. To keep it simple, I modelled three separate types/maps. If we wanted “list all assets,” I'd have stored Asset and have 3 tables for each asset type, and then I would join them.

alexandros-georgantas · 2026-02-03T08:52:22Z

+	return page, total, nil
+}
+
+func (r *FavouriteRepository) Save(_ context.Context, fav domain.FavouriteEntity) error {


Could you describe what will happen if at the same time a user marks an asset as favorite another user deletes that asset? (I've checked that you haven't provided this delete action in the assets repository but lets say that exists). Is the Save implementation protected from that behavior? If not could you suggest how this could be addressed?

For this exercise there is not consistency between the favorites and assets store. Which it will lead to an orphan favorite. 1 option is to add a getAsset while saving the favorite, and a hook when deleting an asset to delete the favorite, but again not ideal. In the real world, if assets and favorites live in the same DB, then we can use a foreign key. and depending on the business rules, either to reject deletion or cascade the delete to favorites.
In case of separate storage and service, again base on the requirements, we can either use soft-deletes and then give time to user to recreate the asset and have the ownership on it. which is nice because things are not suddenly disappear from the users dashboard. or to emit an event from Asset and favorite service to subscribe to that event and clean up the favorite. That accepts eventual consistency but guarantees a valid state. Another option can be SAGA. but I think for this case eventual consistency is good.

alexandros-georgantas · 2026-02-03T08:55:15Z

+
+type Middleware func(http.Handler) http.Handler
+
+func Chain(h http.Handler, m ...Middleware) http.Handler {


could you explain what is the purpose of this function?

In this exercise, it is not useful as I have only one Middleware. If I wanted to add a logging middleware for example the reverse func helps to execute the middlewares in the same order that they are listed. So if the developer want to log->auth->handle then it writes
api.Use(LoggingMiddleware)
api.Use(skipAuthForPaths(jwtCfg, "/health")) which has a result logging(auth(handler))

lkslts64

@emanonk Thanks for submitting the challenge, we really appreciate it!

Your project follows a clear architecture, a decent amount of tests, and solid security practices 💯

Below, I left some comments for you to check.

Thanks

lkslts64 · 2026-02-04T12:19:36Z

+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	out := make([]domain.FavouriteEntity, 0)
+	for _, f := range r.data {


Assuming that the GET /v1/favourites API is heavily used by clients, would you still have r.data map using favourite ID as the map key? What are the reasons for choosing favourite ID as your map key?

You are right, favourite ID as a key wont be efficient for this case. I initially used favorite ID as the key because it’s the natural identifier for update/delete operations with O(1). But given that requirement I could have used map[userId]map[favId]Favourite, as we alwyas lookup for specific user favorites. In real world this could be an secondary index, and in save we can have a unique constrain on user_id & asset_id.

lkslts64 · 2026-02-04T12:20:04Z

+)
+
+type FavouriteRepository struct {
+	mu   sync.RWMutex


Nice use of RWMutex ❤️

lkslts64 · 2026-02-04T12:20:57Z

+
+type FavouriteRepository struct {
+	mu   sync.RWMutex
+	data map[string]domain.FavouriteEntity


If you were to use persistent storage (e.g. a database), which technology would you pick and why?

I think the assets and the favorites makes a lot of sense to be in the same service. I would prefer MySQL or Prostgres as favorite is a reference table to assets. (easy to Join tables). But Mongo can also work and I would considered it more if we had to store asset snapshots or if we wanted a more flexible asset schema. Another factor is always the team familarity, but thats another discussion.

lkslts64 · 2026-02-04T12:22:09Z

+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	if err := server.Shutdown(ctx); err != nil {


Great job with the graceful shutdown 💯

emanonk added 20 commits December 23, 2025 00:38

initial commit, hex assets and favourites, common route

f2ed51c

added jwt, removed userid from router, added add favourite endpoint

51869c9

added delete fav endpoint

6f15490

added logs for debugging in handlers, services and repos

f1b69b4

added update favourite desctription endpoint

56564a3

add mu.lock in inmemory repos for goroutine-safe

dfdab3f

fixed fav-delete unlock

97b45ef

added pagination for user favourites endpoint

ccfcef8

fix pagination on findFavsByUser and fetch assets in batches

5639488

added unit and e2e tests

3ede4e0

added swagger and openapi gen

6b45935

generated files from openapi gen

b9106a0

add dockerfile and dockerignore

e315df8

update README file

48be5da

added config layer

03b92bb

asset not found error handling in asset client fix

3618cf3

added validation for duplicate fav, added swagger UI, added config in…

7e6fbe4

…jection based on env

indirect dependencies

3cf1c0a

change type from string to favouriteType

bf926bb

added api version

147aeac

alexandros-georgantas reviewed Feb 3, 2026

View reviewed changes

lkslts64 reviewed Feb 4, 2026

View reviewed changes


		type Middleware func(http.Handler) http.Handler

		func Chain(h http.Handler, m ...Middleware) http.Handler {

Conversation

emanonk commented Feb 2, 2026

Uh oh!

alexandros-georgantas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lkslts64 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants