Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Luis Dourado Submission #317

Open
wants to merge 141 commits into
base: main
Choose a base branch
from
Open

Luis Dourado Submission #317

wants to merge 141 commits into from

Conversation

lutefd
Copy link

@lutefd lutefd commented Aug 8, 2024

This is my submission for the challenge bravo. This challenge was done in Golang and I'll be happy to have it reviewed.

I included an API KEY made only for the purpose of evaluating the project in the .env.sample and in the readme although it isn't the best I didn't have other means of sending it.

I used the https://openexchangerates.org/ api to fetch the rates.

This commit adds the initial project structure along with the initial
configuration of the server initialization with graceful shutdown. It
includes the following:

- Added a commons package with some JSON wrapping utility functions.
- Initialized the model package (currently empty).
- Created a Dockerfile and Docker Compose setup for the Redis database
(for caching) and the PostgreSQL database (for persistent storage).
- Added a readiness endpoint for health checks.
This commit adds the data models for the currency and exchange rate
entities
This commit adds the initial Currency Repository interface with the
following methods:

- GetByCode: will receive the currency code and return its value or an
error.
- Create: will receive the currency data and create the currency entry
and will return either nil or an error.
- Update: will receive the currency data and update a currency and will
return either nil or an error.
- Delete: will receive the currency code delete a currency entry and
will return either nil or an error.

This is an initial layout of the currency repository, it could change
during the development proccess
This commit adds the initial implementation of the currency service with
the following methods:

- Convert: Converts an amount from one currency to another.
- getRate: Retrieves the exchange rate for a given currency, using the
cache, repository, or external API as needed.
- AddCurrency: Placeholder for adding a new currency (not yet
implemented).
- RemoveCurrency: Placeholder for removing an existing currency (not yet
implemented).
This commit adds the initial implementation of the currency handler with
the following methods:

- ConvertCurrency: Handles HTTP requests to convert an amount from one
currency to another.
- AddCurrency: Placeholder for handling HTTP requests to add a new
currency (not yet implemented).
- RemoveCurrency: Placeholder for handling HTTP requests to remove an
existing currency (not yet implemented).
This commit defines the cache interface with the following methods:

- Get: Retrieves a value from the cache by key.
- Set: Stores a value in the cache with a specified expiration time.
- Delete: Removes a value from the cache by key.
…face

This commit creates the worker package that will interact with the
external exchange rate api and fetch it in the background.
This commit adds the implementation for loading server configuration
from environment variables with the following fields:

- PostgresConn: Connection string for PostgreSQL database.
- RedisAddr: Address for the Redis server.
- RedisPass: Password for the Redis server.
- ServerPort: Port on which the server will run.
…truct

This commit refactors the server initialization to use a configuration
struct, removing the direct environment variable fetching. Changes
include:

- Using the Config struct to pass server configuration.
- Removing the direct environment variable fetching for the server port
within NewServer.
- Incorporating the config parameter in the NewServer function for
cleaner initialization.
This commit refactors the main function to load server configuration
using the LoadConfig function.
This commit adds the currency routes and their corresponding handlers to
the server. Changes include:

- Importing the service package for initializing CurrencyService.
- Initializing CurrencyService and CurrencyHandler in the
loadCurrencyRoutes function.
- Adding the following routes to the server:
- GET /convert: Route to convert currency.
- POST /: Route to add a new currency.
- DELETE /{code}: Route to remove an existing currency.
This commit renames the file redis_cache.go to cache.go since it's only
defining the cache interface and I will use the redis_cache file to
implement the caching through redis.
This commit updates the go.mod and the go.sum files to include
additional dependencies for the PostgreSQL and Redis client libraries,
along with tidying up the files.
This commit enhances the configuration loading by adding error handling
and validation for required environment variables. Changes include:

- Adding the APIKey field to the Config struct.
- Modifying LoadConfig to return an error if required environment
variables are not set.
 - Adding validation for REDIS_PASSWORD, REDIS_ADDR, POSTGRES_CONN,
API_KEY, and SERVER_PORT environment variables.
- Returning detailed error messages if any configuration values are
missing or invalid.
This commit enhances the main function by adding error handling for
configuration loading. Changes include:

- Logging an error message and terminating the program if configuration
loading fails.
- Updating the server initialization to use the loaded configuration.
- Ensuring proper error handling in the server startup process.
This commit adds the implementation of the PostgreSQL currency
repository with the following methods:

- NewPostgresCurrencyRepository: Initializes a new
PostgresCurrencyRepository with a given connection URL.
- GetByCode: Retrieves a currency by its code from the database.
- Create: Inserts a new currency into the database.
- Update: Updates an existing currency in the database.
- Delete: Deletes a currency from the database by its code.
This commit adds the implementation of the Redis cache with the
following methods:

- NewRedisCache: Initializes a new RedisCache with a given address and
password.
- Get: Retrieves a value from the cache by key, returning an error if
the key is not found or if retrieval fails.
- Set: Stores a value in the cache with a specified expiration time,
returning an error if the operation fails.
- Delete: Removes a value from the cache by key, returning an error if
the operation fails.
This commit adds the implementation of the OpenExchangeRates API client
with the following methods:

- NewOpenExchangeRatesClient: Initializes a new OpenExchangeRatesClient
with a given API key.
- FetchRates: Fetches the latest exchange rates from the
OpenExchangeRates API, handling HTTP requests and responses, and returns
the exchange rates.
This commit adds the implementation of the rate updater with the
following functionality:

- NewRateUpdater: Initializes a new RateUpdater with the given
repository, cache, external API client, and update interval.
- Start: Begins the periodic updating of exchange rates using a ticker
and context for graceful shutdown.
- updateRates: Fetches the latest exchange rates from the external API
and updates the repository and cache with the new rates.
This commit adds the implementation for the AddCurrency and
RemoveCurrency endpoints in the CurrencyHandler.
This committ defines and implements a Close method on the Cache
interface for closing the connection to redis.
This commit define and implements the Close method on the currency
repository interface for closing the Postgres Connection.
This commit adds a centralized logging utility with separate loggers for
informational and error messages. The following functionalities are
included:
- Info: Logs informational messages.
- Infof: Logs formatted informational messages.
- Error: Logs error messages.
- Errorf: Logs formatted error messages.

Both loggers include date, time, and short file information in their
output.
This commit refactors the server setup. Changes include:
- Initializing repository, cache, and external API client within the
NewServer function.
- Adding rate updater initialization and starting it in a separate
goroutine.
- Enhancing the Start function to log server start and handle HTTP
server errors.
- Implementing a Shutdown function to gracefully shut down the server,
close database and cache connections, and log shutdown progress.
- Utilizing the logger package for logging server events and errors.
This commit refactors the route registration process to streamline the
initialization of currency routes. Changes include:

- Passing the CurrencyService instance to the registerRoutes function.
- Consolidating the route definitions within the registerRoutes
function.
- Removing the separate loadCurrencyRoutes function for better
readability and maintenance.
This commit improves error handling and server initialization in the
main function. Changes include:

- Logging an error message if the .env file fails to load.
- Handling errors when creating the server and logging appropriate
messages.
- Logging an error message if the server fails to start.
- Ensuring consistent error logging and handling throughout the
initialization process.
This commit adds the implementation for the AddCurrency and
RemoveCurrency methods in the CurrencyService. Changes include:

- AddCurrency: Adds a new currency to the repository and updates the
cache. Returns an error if the currency already exists.
- RemoveCurrency: Removes an existing currency from the repository and
cache. Returns an error if the currency is not found.
- Proper error handling and logging for cache updates and deletions.
This commit adds the User model to the project. The User model includes
the following fields:

- ID: Unique identifier for the user.
- Username: The username of the user.
- Password: The hashed password of the user (excluded from JSON
responses).
- Role: The role of the user, which can be either 'user' or 'admin'.
- APIKey: The API key associated with the user.
- CreatedAt: The timestamp when the user was created.
- UpdatedAt: The timestamp when the user was last updated.
This commit enhances the RateUpdater by adding an initial rate
population on startup. Changes include:

- Adding a populateRates method to fetch and update rates from the
external API on startup.
- Calling populateRates at the start of the Start method to ensure rates
are up-to-date when the server starts.
- Improved error handling and logging within the populateRates method to
handle cases where currencies are not found or updates fail.
This commit adds the UserRepository interface to the project. The
UserRepository interface includes the following methods:

- Create: Adds a new user to the repository.
- GetByUsername: Retrieves a user by their username.
- GetByAPIKey: Retrieves a user by their API key.
- Close: Closes the repository connection.
This commit updates the go.mod file to include the
go-scalar-api-reference module for serving the Swagger API documentation
as HTML.
Updated Dockerfile to include Swagger documentation in the Docker image.
…iables

Improved the sample environment configuration by separating the
PostgreSQL connection string into individual environment variables.
This commit removes the unused update method of the user service. This
method was written because I was going to make a more complex user
management system, but as the application grew I tried to simplify
things and leave only what was necessary to run this project.
This commit adds the missing test for the GetByAPIKey method of the user
service.
This commit adds better quality images of the c4 model and the entity
map
This commit adds the dependency map mermaid file and the diagram
generated from it.
This commit adds a postman collection to help evaluators manually test
the api endpoints.
This commit changes the testing command removing the -v flag for a less
verbose testing output
Updated the README.md to include detailed documentation for the Bravo
Challenge API. The README now contains sections on project description,
technology stack, services, features, setup and configuration, API
documentation, and error responses.

Changes:

- Removed README.pt.md.
- Expanded README.md with detailed instructions and examples for setting
up and using the API.
- Added information on environment variables and constants
configuration.
- Included examples for Docker setup, local development, and testing.
This commit fills out the pull request file.
This commit changes the docker compose to have the redis port hard
setted to streamline setup and avoid another environment variable that
is used just there.
…luators

This commit sets the API_KEY env with an api key created specificly to
streamline the evaluators setup, so they don't need to generate one in
https://openexchangerates.org/. This is a practice that wouldn't happen
in production code, but as this is a challenge for a job opportunity I
thought it was best to do this.
…code, improvements, and final thoughts

Added new sections to README.md to provide additional context and
insights about the project. The updates include:

- A section on auto-generated code explaining the use of tools like
goose, mermaid, and swagger.
- A section on potential improvements that could be made to the project,
discussing rate limiting, logging, rate updater, currency management,
user management, and testing.
- A final thoughts section reflecting on the learning experience and
future improvements.
This commit fixes a mistake I made when commenting about the API_KEY in
the .env.sample so if the evaluator only copied without removing the
comment it would error out
… caching

his commit introduces improvements to the CurrencyService to handle
in-flight request deduplication and negative caching to enhance
performance and try to avoid rate limit on API.

Changes:
- Added in-flight request deduplication using sync.Map to prevent
duplicate requests for the same currency code.
- Implemented negative caching to cache failed currency lookups,
reducing the load on the external API.
- Updated `getRate` method to handle in-flight deduplication and
negative caching.
- Added related constants to `constants.go`.
- Included new test cases in `currency_service_test.go` to verify the
functionality of in-flight deduplication and negative caching.

Also, constant `CacheExpiration` was added to manage cache duration
settings.
…dependency

This commit simplifies the `CurrencyService` by removing the external
API client dependency. The service now relies solely on the repository
for currency data, streamlining its responsibilities and reducing
complexity.

Changes:
- Removed the external API client from `CurrencyService` and related
methods.
- Updated the `CurrencyService` constructor and methods to remove
external API interactions.
- Modified corresponding tests to reflect the removal of the external
API client.
- Updated the server setup to match the new `CurrencyService`
constructor.

This refactor aims to mitigate the external api limits. It would be nice
to be able to check there for missing currencies, but the limits are too
low and even with negative caching and request deduping it's too risky.
…urrencyLength` constant

Added clarification to the README regarding the `AllowedCurrencyLength`
constant. Users are now informed that changes to this constant also
require updating the `005_expand_currency_code.sql` migration, because
there we set the column type as CHAR(5).
This commit removes a typo in the end of the readme file
…stants

This commit enhances the rate updater by using the logger to handle the
error logs of the job. Other changes were cleaning up code smell and
updating the docs, as it follows:

- Added `RateUpdaterInterval` constant to `constants.go` to allow an
easiear and more readable configuration of the rate updater interval.
- Updated the Rate Updater initialization to utilize this new constant.
- Replaced `log` with `logger` for improved logging consistency
throughout the `RateUpdater`.
- Updated the README to document the new `RateUpdaterInterval` constant.
This commit moves the config functionality from the server package to
the commons package, as the seeding and rate updating were moved into
separate services it wasn't only the server using the config anymore and
it made sense to move it to the commons package. The changes are just
changes of which package is being called
This commit moves the worker functionality to its own service, it makes
more sense than being ran in the background of the server, if this
should scale to a distributted system you would have multiple workers
running, one for each server, which is nonsense.
…change

This commit updates the README to reflect the changes on the refactoring
of the confing and the improvement of the worker to its own service. It
also removes some repeated text from when I was composing the readme
from different notes.
…ious changes

This commit updates the c4 model and the dependency map to reflect
previous changes on the project.
This commit along with fixing the table of contents, adds the
documentations diagrams pngs into the readme itself.
This commit adds a heartbeat mechanism to help confirm it's working,
this could evolve into a health check mechanism that would be checkable
in the docker compose for container health.
…ments

This commit adds the WorkerHeartbeatInterval constant to the config docs
and adds a few points of improvements that could be done in the rate
updater and its service
This commit adds instructions for getting the ADMIN API Key created in
the seed proccess along with the migrations. Evaluators should use them
to test the protected endpoints.
THis commit removes comments related to test on the worker where it
detailed that it was the same mock as before but with some added for
debugging it.
…ogue

This commit adds the show_alternative flag in the openexchangerate API,
I noticed in some manual tests that the ETH currency along with some
other ones were missing in the API response without the flag.
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.

1 participant