Challenge implementation

[ba091724]

No description provided.

[lunemec]

Hello @ba091724 ,
thank you for your submission.

My first question on your submission is, what will happen if I take the code, and deploy it to production, and have few concurrent users using it? Can you describe what will happen, in as much detail as you can think of?

Thank you

[ba091724]

Hello @lunemec ,
Deploying this version to production and exposing it to users firing multiple requests concurrently may lead to a memory leak due to failing to call the context canceling function.
The problem is that when initializing the components in configs#NewConfigs , the returned function that cancels the context when creating a connection to the database is ignored:

ctx, _ := context.WithTimeout(context.Background(), 10*time.Second)
client, err := mongo.Connect(ctx, options.Client().ApplyURI(fmt.Sprintf("mongodb://%s:%s@%s:%s/?authSource=%s", dbUser, dbPass, dbHost, dbPort, dbName)))
// more code ...

Here the context is created with a timeout of 10 seconds. This means that a database connection will open and remain alive for at least 10 seconds when a request is received, regardless of when it will be completed. Since the implemented requests are simple and their response time is less than 1 second, it is safe to assume that 10 seconds of timeout is not an efficient threshold, and of course not calling the cancel function in a defer section is a miss.

[lunemec]

@ba091724 can you spot any potential issues with this function? If yes, what would they be, what could they cause, and how would you fix them?

Hint: there are multiple

[ba091724]

There are at least two main issues related to the way this method deals with slices:

1. There is no need to use make to initialize and return a new empty slice (line 48). I could have used return nil instead, since I am finding out that the zero value for a slice is nil after all.
2. In line 50 there is another slice initialization for len(ufs) elements length. However, since those elements are populated only if ApiService#getAsset finds the requested assets, there is an open chance for an asset to not be found. This will lead to a slice being initialized to hold more elements than it actually holds.

Some additional comments regarding this method:

• Creating an else block in line 56 is not necessary. It only increases the cognitive complexity of this method.
• It is recommended to also return an error in the method signature, since an error might occur in case ApiService fails to find an asset.
• Fetching the assets one by one might not be ideal. In case of many assets, the service will execute multiple requests, leading to multiple database queries. This could be optimized by writing a batch query. Something like (SQL for example) SELECT * FROM asset WHERE id IN (...).