Favourites API Solution - Katerina Kourti

[kkourti]

No description provided.

[dzacharakis]

Hi @kkourti 👋

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]

userID is always a path parameter string (/users/{userID}/...) and is never validated/loaded as an entity.

What would the User model look like and where would it live (package-wise)?

[kkourti]

userID is intentionally treated as an opaque external identifier. As described in the Assumptions section of SOLUTION.md, users and assets originate from upstream systems, and authentication/authorisation are handled externally. This service is responsible only for: managing favourite relationships, enforcing uniqueness (user_id, asset_id), handling pagination and caching. It does not own: user lifecycle, authentication, authorization, user validation.
In a production architecture, requests would typically flow:
Client → API Gateway → Identity/Auth Service → Favourites API
The identity service would authenticate the user, issue a JWT/OAuth token, embed the validated user_id in the sub claim, and enforce RBAC policies.
A simplified identity model could look like:

type User struct {
ID string
Email string
Username string
PasswordHash string (Argon2id or bcrypt)
CreatedAt time.Time
}

type Role struct {
ID string
Name string
}

type UserRole struct {
UserID string
RoleID string
}

RBAC would allow restricting asset access or certain operations (e.g. updates) based on roles. The favourites service relies on those upstream guarantees and intentionally preserves separation of concerns.

[dzacharakis]

It seems that there is no global Asset entity shared across users. Each user has his own Asset instance in memory, right?

[kkourti]

Yes that’s correct, and it’s intentional for this implementation. Each user has their own favourite record keyed by asset_id. The stored value is the asset payload as it was favourited, including favourite-specific fields like description and favourited_at.
This means:

• There is no global asset catalogue in this service.
• Two users can favourite the same upstream asset_id, but each favourite is an independent record (separate description, separate favourited_at).
• Caching is also per-user (favourites:user:), which aligns with the primary access pattern (dashboard reads are user-scoped).

[dzacharakis]

Here, conflict check is scoped per user (userID -> assetID) so the same asset can be favourited by multiple users, which is correct behaviour for favourites. However, because there is no separate Asset store or Favourite entity, the system ends up duplicating asset payloads per user and cannot enforce or reason about global asset existence/identity.

[kkourti]

Yes, correct observation. In this implementation the conflict check is per user (user_id, asset_id), so multiple users can favourite the same upstream asset, which matches the domain. Also, due to the fact that this service does not own a global asset catalogue (assets are assumed to come from upstream systems), we store the “favourited snapshot” payload per user. That means the payload can be duplicated across users and we can’t enforce global asset existence inside this service. This is an intentional scope decision based on the assignment assumptions: asset_id is treated as an external reference, and this service’s responsibility is the favourite relationship + metadata (description, favourited_at, pagination). In production, if we wanted to reason about global asset identity/existence, we’d either:

• store only references + minimal favourite metadata here and fetch asset details from an upstream Asset service, or
• maintain an assets table and have favourites reference it (still with a (user_id, asset_id) uniqueness constraint).

So the current design optimises for correctness of the favourites domain and simplicity (single service, no cross-service calls), while keeping a clear path to a global asset model if the platform requires it later.

[dzacharakis]

Handlers work directly with domain models (models.Asset) rather than dedicated DTOs. For this small service it works, but at platform scale it tightly couples the HTTP contract to internal types and would be risky for entities like User that have sensitive fields.

How would you introduce DTOs here without breaking the current tests?

[kkourti]

I completely agree with the concern regarding domain - transport coupling. While unmarshaling directly into models.* is efficient for a service of this scope, it creates risks at platform scale for API versioning and security (e.g., leaking internal fields).
Regarding the User entity and general asset handling, adopting DTOs provides an explicit serialization boundary. Even though this service treats user_id as an opaque identifier provided by upstream systems, this boundary ensures that any internal fields we might add to our domain models in the future are physically prevented from being exposed in the API response. It allows the service to maintain a stable public contract regardless of how our internal data structures evolve.
To introduce DTOs without breaking current tests or the JSON contract I would:

1. Introduce a DTO Layer: Create a dedicated package (e.g., internal/httpdto) to house Request and Response structs. These structs will own the JSON tags and validate tags, keeping the models package "pure."

2. Implement the Mapper Pattern: I would add mapping functions to bridge the layers:

• ToDomain(): Converts validated DTOs into the models.Asset interface.
• FromDomain(): Converts the domain interface back into response DTOs.

3. Implement Factory Pattern for Polymorphic Assets: Since assets have different structures, the DTO layer would handle the initial unmarshaling of the Type field to determine which specific DTO struct (e.g., CreateAudienceRequest) to use before mapping it to the domain.

4. Preserve the Public Contract: By keeping the JSON field names in the DTOs identical to the current implementation, all existing integration tests, curl scripts, and Swagger documentation remain valid.

5. Isolate the Service: The favouritesService and the store package will continue to operate on models.Asset. This ensures the business logic remains decoupled from HTTP-specific changes.

[dzacharakis]

You’ve abstracted Storage to allow in‑memory or database-backed stores.

Can you walk me through how you would implement a relational database-backed store? What would the schema look like?

[kkourti]

In a production scenario, I’d lean towards a relational database, specifically PostgreSQL to handle the uniqueness constraints and the ordered indexing required for the keyset pagination. I actually detailed a specific implementation path for this in the Storage Design section of the SOLUTION.md (Option A/B).

The implementation would use a favourites table where core queryable fields like user_id, asset_id, and favourited_at are first-class relational columns, while the polymorphic asset data (the unique fields for Charts vs. Audiences) is stored in a JSONB column. To keep response times "reasonable" and pagination at O(1), I would use a composite index on (user_id, favourited_at DESC, asset_id DESC). This ensures our cursor-based pagination performs as a direct index seek regardless of the dataset size.

I would implement this via a PostgresStore struct that satisfies the FavouriteStore interface. The Storage abstraction allows us to swap the in-memory store for PostgreSQL at the entry point without touching any business logic or HTTP handlers.

This approach actually highlights the value of the DTO/Mapping pattern we discussed earlier. The repository would unmarshal the database row into a persistence-specific DTO before mapping it to the models.Asset interface. By using this approach, our internal business logic remains shielded from the underlying storage format. If we ever needed to migrate to a fully normalized schema (like Option C) to enforce stricter data integrity, we would only need to update the repository's mapping logic while the rest of the application remains untouched.

[dzacharakis]

Here, logging helpers mix Errorw (structured) with Warnf/Errorf (printf-style) but still pass key/value pairs. With Zap, Warnf/Errorf expect a format string, so those calls won’t produce the intended structured fields.

I would standardize on Warnw/Errorw with key/value pairs for consistency and correct structured logs.

[kkourti]

That makes total sense. Coming from Python, I'm still calibrating to Zap's strict distinction between printf and structured interfaces. I see now that I was mixing the two, using Warnf naming while passing key-value pairs.

[lkslts64]

Hey @kkourti, thanks for submitting the engineering challenge.

I really enjoyed reading your work, especially the well-written README 💯 Great stuff!

Below, I left a comment for you to check. Please have a look when you find some time.

[lkslts64]

For pagination you’re still loading the full list for the user and then trimming to a page. How would you see this evolving if we had a real database and wanted to avoid loading everything for each request?

[kkourti]

In the current in-memory implementation, loading the full list is a constraint of using a map, which is unordered and requires a full scan/sort to identify a specific range.

With a real database, the GetFavouritesPaginated service would pass both the decoded cursor and the limit directly to the repository. The repository would then execute a keyset pagination query:

SELECT * FROM favourites
WHERE user_id = $1
AND (favourited_at, asset_id) < ($2, $3) -- cursor seek
ORDER BY favourited_at DESC, asset_id DESC
LIMIT $4; -- limit

By leveraging the composite index on (user_id, favourited_at, asset_id), the database performs a direct index seek to the cursor position and retrieves only the requested number of rows. This ensures the app's memory footprint remains constant O(1) regardless of the total number of favourites a user has.

[dzacharakis]

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