Implement Favorites API with validation, tests and Docker support by kongrillas · Pull Request #69 · GlobalWebIndex/platform-go-challenge

kongrillas · 2026-02-02T09:07:16Z

Summary

This PR implements a lightweight HTTP API that allows users to manage a per-user list of favorite assets.

The solution focuses on correctness, clarity, and pragmatic design choices, while keeping dependencies minimal and the codebase easy to reason about.

Features

REST API for managing user favorites (list, add, update description, remove)
Support for multiple asset types:
- Insight
- Chart
- Audience
Type-specific payload validation per asset type
Concurrency-safe in-memory storage using RWMutex
Handler-level tests using httptest
Multi-stage Dockerfile for containerized execution

Design Notes

No external routing or validation libraries were used to keep dependencies minimal.
Asset payloads are validated via type-specific structs instead of rigid schemas, allowing flexibility for future extensions.
In-memory storage was chosen for simplicity; the design allows easy replacement with persistent storage.

dzacharakis

Hi @kongrillas 👋

Thanks for your work on this assignment!

I added some comments on your PR with a few questions. I’d love to hear your feedback when you get a chance.

Thanks,
Dimitris

dzacharakis · 2026-02-04T15:11:40Z

+	ID          string          `json:"id"`
+	Type        AssetType       `json:"type"`
+	Description string          `json:"description,omitempty"`
+	Payload     json.RawMessage `json:"payload"` // type-specific


You’re using json.RawMessage for this polymorphic field. What are the drawbacks of this approach?
Can you suggest a better alternative?

Using json.RawMessage allows deferring payload decoding until the asset type is known, which keeps the initial model flexible and helps to avoid "premature" coupling. So, for this task, I chose RawMessage, in order to keep the design as simple and flexible is possible, but having correct approach though every validation per asset type.

The main drawbacks, I can think of are:

Loss of compile-time type safety for the payload field.

Validation that will rely on runtime checks and will happen later than it should.

Making refactoring weaker.

As better alternative, I can think to model the payload as an interface and use a custom UnmarshalJSON, based on the asset type.

dzacharakis · 2026-02-04T15:24:52Z

+	"github.com/kongrillas/platform-go-challenge/internal/http/handlers"
+	"github.com/kongrillas/platform-go-challenge/internal/http/router"
+	"github.com/kongrillas/platform-go-challenge/internal/store"


Why did you choose internal? Is it because you are following a standard project/directory layout or other reason?

I used the internal layout to clearly communicate package boundaries and prevent any unneeded reuse of application internals outside the module. Because in my example, HTTP handlers, models and storage are implementation details of the service and I do not want to be used by external consumers.

dzacharakis · 2026-02-04T15:30:48Z

+	app := router.New(h)
+
+	fmt.Println("listening on :8080")
+	_ = http.ListenAndServe(":8080", app)


Can you explain the choice of using the blank identifier here?
The error is intentionally ignored/ silently discarded—what makes that the right decision, and is there a better alternative?

The blank identifier was used to keep the example minimal and focused on the API logic. Of course, I get it is not ideal, because in a production system, we should have graceful shutdown, context cancellation and a proper error propagation. As better approach I would recommend something that logs the error and exit the process or wrap ListenAndServer like that:

log.Fatal(http.ListenAndServe(":8080", app))

dzacharakis · 2026-02-04T15:41:54Z

+)
+
+type FavoritesHandler struct {
+	Store *store.MemoryStore


What are possible drawbacks of using a concrete *store.MemoryStore?

First of all, like that we reduce flexibility and testability.

The main drawbacks are:

It is harder to swap storage implementations (for example: database-backed store)

Tighter coupling between HTTP layer and persistence

Mocking requires concrete types instead of interfaces

A more flexible approach would be to depend on a Store interface that defines the required behavior, allowing different implementations (in-memory, database, cache-backed) without changing the handler logic.

For this exercise, I chose the concrete type for simplicity and clarity, but introducing an interface would be a natural next step.

dzacharakis · 2026-02-04T15:45:54Z

+func writeJSON(w http.ResponseWriter, status int, v any) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(status)
+	_ = json.NewEncoder(w).Encode(v)


What will happen here if JSON encoding fails?

If JSON encoding fails, the error is currently ignored and the response may be partially written or invalid. Of course, this cannot be used in a production system where a more suitable approach would be to log the error, avoid writing the body, when the headers are not sent yet and return an internal server error when it's possible.

In a real-world service proper error handling would be required, I used this approach here assuming encoding failures are unlikely given validated input.

dzacharakis · 2026-02-04T15:47:55Z

+	"strings"
+)
+
+var ErrInvalidPayload = errors.New("invalid payload")


What are the downsides of returning the same error for every validation failure?

Returning a single generic validation error simplifies the API but reduces observability and the feedback that you can receive from the client or anyone, who uses it. So, it is harder to debug, you lose field-level error information and it's difficult to distinguish different failure causes.

I chose the simpler way for this challenge, to keep the validation logic straght-forward and in general to avoid over-engineering to the whole project.

dzacharakis · 2026-02-04T15:58:32Z

+	_ = rr // silence unused if kept
+	_ = req


Have you used a linter, and did you explicitly silence the type-check warnings to handle those cases?
Here it doesn’t seem necessary (wouldn’t it be more appropriate to silence line 68 NewRecorder()?),
but do you know of another way to handle linter issues?

I didn’t run a linter explicitly for this task, but I’m familiar with common Go linters such as golangci-lint and staticcheck.
In this case, the blank assignments were used to silence unused variables introduced for test clarity.

Alternative approaches could include:

To restruct test to avoid unused variables

To limit the variable scope

To silence warnings closer to the actual source (as you suggested)

In production code, I would prefer restructuring over silencing wherever possible.

pamanta

Hi @kongrillas,

Thank you very much for taking the time to complete the assignment.

I have also added a couple of comments for you to take a look when you have the time

pamanta · 2026-02-05T07:34:21Z

+	mux.HandleFunc("/users/", func(w http.ResponseWriter, r *http.Request) {
+		switch r.Method {
+		case http.MethodGet:
+			h.ListFavorites(w, r)
+		case http.MethodPost:
+			h.AddFavorite(w, r)
+		case http.MethodDelete:
+			h.RemoveFavorite(w, r)
+		case http.MethodPatch:
+			h.UpdateDescription(w, r)
+		default:
+			http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+		}
+	})


Can you identify any drawbacks or issues this routing mechanism might have? If yes, how would you fix them?

The current routing is intentionally minimal (stdlib ServeMux + switch r.Method), but I can see some drawbacks:

ServeMux doesn’t provide first-class path params, so parsing {userId} / {assetId} becomes manual and error-prone as routes grow.

Switch per handler scales poorly as endpoints expand, and it’s easy to miss consistent behavior.

Adding auth, logging, request IDs, etc. becomes repetitive without a router chain.

As the API grows, it’s harder to understand the routing table and enforce consistent patterns.

In a production setting, I’d likely use a lightweight router (e.g. chi) to get clean path params, method routing, and middleware chaining out of the box. In both cases I’d add consistent error handling + structured logging + request-scoped context.

pamanta · 2026-02-05T07:46:52Z

@@ -0,0 +1,18 @@
+package model


If you were to add persistence, which type of database would you use, why would you choose it, and how would you model the schema?

For persistence I’d likely choose PostgreSQL, for its strong consistency and because it fits to favorite lookups. Also it's easy to model uniqueness (no duplicates per user+asset), indexing, and transactional updates.

For polymorphic payload, Postgres JSONB is a pragmatic middle ground: flexible schema + queryable if needed.

For a heavy production project:

favorites(user_id, asset_id, type, description, created_at, updated_at)

type-specific tables:

favorite_insights(asset_id, text, …)

favorite_charts(asset_id, title, axes, data, …)

favorite_audiences(asset_id, …)

For this task I used an in-memory store for simplicity, but Postgres + JSONB with a composite key and proper indexing would be my default production choice.

dzacharakis · 2026-02-06T08:26:27Z

Thank you Konstantinos for your responses to our comments.
All good on my end. Good luck with the next steps!

kongrillas · 2026-02-06T08:40:05Z

Thank you Konstantinos for your responses to our comments. All good on my end. Good luck with the next steps!

Thanks a lot too Dimitris, looking forward for the next steps!

konstantinosgkrillas and others added 4 commits January 30, 2026 17:26

Initialize Go module and server skeleton

56649c9

Validate asset payloads and add handler tests

f7cd5b5

Add Docker support and finalize README

9d96232

Finalize Readme File

778229f

dzacharakis reviewed Feb 4, 2026

View reviewed changes

pamanta reviewed Feb 5, 2026

View reviewed changes

Conversation

kongrillas commented Feb 2, 2026

Summary

Features

Design Notes

Uh oh!

dzacharakis 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!

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!

kongrillas Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pamanta 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!

dzacharakis commented Feb 6, 2026

Uh oh!

kongrillas commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kongrillas Feb 4, 2026 •

edited

Loading