Skip to content

implementation of GWI go challenge#68

Open
steliosant wants to merge 12 commits into
GlobalWebIndex:mainfrom
steliosant:main
Open

implementation of GWI go challenge#68
steliosant wants to merge 12 commits into
GlobalWebIndex:mainfrom
steliosant:main

Conversation

@steliosant

Copy link
Copy Markdown

Implementation of GWI go challenge by Stelios Antonopoulos

@NikosMas NikosMas left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @steliosant 👋
Thanks again for the time and effort you put into this project — I really enjoyed reviewing it.

Happy to see:

  • Great documentation overall (Swagger), plus the Docker setup was very clear.
  • Nice touch adding PostgreSQL initialization.
  • Solid test coverage.
  • Clean auth management; the middleware usage was a big plus.

A few questions/discussion points:

  • I noticed there’s quite a bit of business logic inside the HTTP handlers. How would you approach keeping handlers cleaner if you were to iterate on the design?
  • Was there a specific reason you chose mux for the HTTP server?
    Also, what was the reasoning behind using string splitting to determine parts of the URL path?
  • If you had more time to deliver the project, what would you add or change?
  • At a high level, what steps would you follow to deploy this service to a production Kubernetes cluster?

Thanks!
Looking forward to your thoughts!

@apopsallas apopsallas left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @steliosant !

Thank you for your time and effort on this assignment, and for putting together the Swagger setup, Docker configuration, and DB initialization. Those were especially helpful and appreciated.

After reviewing the project, I had a few questions about your implementation choices:

  • Interesting implementation for routing. Could you elaborate on why you chose to use string manipulation instead of mux’s built-in routing functionality?

  • I noticed the DB connection is passed from main through all layers and into the repositories as a function parameter. Could you share your thoughts on this approach versus using constructor-based dependency injection?

  • I also noticed that the description field isn’t part of the Asset model and is instead passed as a separate parameter. Could you walk me through the reasoning behind that design choice?

  • Could you share how you typically think about structuring layers and responsibilities in a larger production Go service?

Looking forward to your thoughts! Thanks again!

@steliosant

Copy link
Copy Markdown
Author

Hi @steliosant 👋 Thanks again for the time and effort you put into this project — I really enjoyed reviewing it.

Happy to see:

  • Great documentation overall (Swagger), plus the Docker setup was very clear.
  • Nice touch adding PostgreSQL initialization.
  • Solid test coverage.
  • Clean auth management; the middleware usage was a big plus.

A few questions/discussion points:

  • I noticed there’s quite a bit of business logic inside the HTTP handlers. How would you approach keeping handlers cleaner if you were to iterate on the design?
  • Was there a specific reason you chose mux for the HTTP server?
    Also, what was the reasoning behind using string splitting to determine parts of the URL path?
  • If you had more time to deliver the project, what would you add or change?
  • At a high level, what steps would you follow to deploy this service to a production Kubernetes cluster?

Thanks! Looking forward to your thoughts!

Hi Nikos! Thanks for the effort you put on reviewing my project. I will try to answer to all your bullets:

I noticed there’s quite a bit of business logic inside the HTTP handlers. How would you approach keeping handlers cleaner if you were to iterate on the design?

Indeed there is quite of a business logic into the handlers. In a different approach for keeping the handlers cleaner I would add a service layer for the business logic. In this layer I would add some logic such as: user validation, request response handling , error Mapping (Converting domain errors to HTTP status codes), HTTP concerns (Status codes, headers, response formatting), Path parsing (Extracting IDs from URL paths). 
This change would have made the code cleaner and would have add some benefits such as reusability and maintainability.

Was there a specific reason you chose mux for the HTTP server?
Also, what was the reasoning behind using string splitting to determine parts of the URL path?

When I started the project(no previous Go experience) “net/http” library was the standard library for doing that. While using it I faced some limitations, for example I couldn’t extract a parameter from a path for example: in “/users/{id}/favourites” was not able to extract {id}.
So the string splitting was a workaround for this limitation.
I did some searching(after yours and your colleague’s comments about string splitting) and I realised that there are some alternative libraries that can do the job, like Gorilla/mux. I believe this would have been probably a better approach.

If you had more time to deliver the project, what would you add or change?
I would do the changes I mentioned in the two previous questions:

  • Refactor handlers into a service layer and add consistent error/response helpers.
  • Use different library for http server, (probably Gorilla/mux ?) so I could avoid string splitting and probably have other benefits as well such as better error handling
  • I would add integration tests
  • I was thinking also to add a Grafana instance for better visualisation of data

At a high level, what steps would you follow to deploy this service to a production Kubernetes cluster?

Preparation

  1. Container image registry setup of my application (something like docker build -t myregistry.io/platform-go-api:v1.0.0 .
  2. Decision about database setup and backups (creation and initialisation).
  3. Create secrets where we need them (usernames, passwords, generic jwt-secret? )
  4. Create Kubernetes manifest files.
    1. Namespace.yaml
    2. Configmap.yaml
    3. Database
    4. Deployment.yaml
    5. Service.yaml
    6. Ingress.yaml
  5. Thinking about monitoring options depending the environment (cloud) I want to deploy.
  6. Set up all environments (dev-cluster, Test-cluster, Production cluster)
  7. Configure CI-CD pipelines (deployment in Test environment first —> run tests (integration? ) —> deployment in production cluster)

@steliosant

Copy link
Copy Markdown
Author

Hello @steliosant !

Thank you for your time and effort on this assignment, and for putting together the Swagger setup, Docker configuration, and DB initialization. Those were especially helpful and appreciated.

After reviewing the project, I had a few questions about your implementation choices:

  • Interesting implementation for routing. Could you elaborate on why you chose to use string manipulation instead of mux’s built-in routing functionality?
  • I noticed the DB connection is passed from main through all layers and into the repositories as a function parameter. Could you share your thoughts on this approach versus using constructor-based dependency injection?
  • I also noticed that the description field isn’t part of the Asset model and is instead passed as a separate parameter. Could you walk me through the reasoning behind that design choice?
  • Could you share how you typically think about structuring layers and responsibilities in a larger production Go service?

Looking forward to your thoughts! Thanks again!

Hi Apostolos! I really appreciate the time you took to review my project! I will try to answer to your questions.

Interesting implementation for routing. Could you elaborate on why you chose to use string manipulation instead of mux’s built-in routing functionality?
I had similar question in your colleague’s comment which made me thought about this choice. In “net/http” library l which I used, I was not able extract a parameter from a path. for example: in “/users/{id}/favourites” was not able to extract {id}. That is the reason I did this workaround with string manipulation. If I had to do it again I would use another approach, probably using another library like go Gorilla/mux.

I noticed the DB connection is passed from main through all layers and into the repositories as a function parameter. Could you share your thoughts on this approach versus using constructor-based dependency injection?
I chose that approach since it's the simplest approach. You can see exactly where the DB comes from at each call site. For a challenge project with a limited scope this works without needing structs or adding more complexity. 
However I could have used constructor-based dependency injection. Instead of passing db to every handler, create a handler struct that holds the database, then define methods on it. This would have some benefits if the projects grows or needs to be deployed in production environment, such as:

  • Much cleaner code (no DB param in every signature)
  • Easier testing (mock entire handler at once)
  • Single point of dependency injection  (Dependencies injected once at construction, not at every call)
  • scales better as you add more dependencies like loggers, caches, or services

I also noticed that the description field isn’t part of the Asset model and is instead passed as a separate parameter. Could you walk me through the reasoning behind that design choice?
The description field is kept outside the Asset model but stored in the assets table because it serves a different purpose depending on context.

  • Asset-level description — A general description of what the asset is (stored in assets.description)
  • Favourite-level description — A personal note about why you favorited it (stored in favourites.description)
    The Asset model is used for:
  • Creating/retrieving assets (business assets, not user-specific)
  • Returning asset data to users

If description was part of Asset, it would be:

  • Confusing which description you're using (asset's or favourite's?)
  • Hard to distinguish asset metadata from user annotations
  • Makes the model larger and less focused

With the current design :

  • Asset stays focused on asset metadata
  • FavouriteAsset clearly combines asset data + user's personal note
  • Database schema matches domain logic (two separate concerns)
  • Easy to extend (e.g., add tags, ratings without polluting Asset model)

Those are my thoughts on that subject, please feel free to share your opinion

Could you share how you typically think about structuring layers and responsibilities in a larger production Go service?
I would layer a larger production Go server like this:

Entry Point (main.go)

  • Config, dependency wiring, server bootstrap

HTTP/API Layer (handlers/)

  • Request parsing, validation, response format
  • HTTP status codes, headers
  • Route definitions, middleware

Business Logic Layer (services/)

  • Domain logic, validation
  • Coordinate across repositories
  • Domain errors, rules, workflows

Data Access Layer (repositories/)

  • Query building, result mapping
  • Raw database operations only
  • No business logic here

Data Models (models/)

  • Domain entities, DTOs
  • No behavior, just data structures

In my current implementation I am mixing business logic in the handlers layer. If I had to implement it again I would have separate it by adding a service layer as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants