guregu / dynamo

expressive DynamoDB library for Go
Other
1.31k stars 179 forks source link

Replacing struct references with interface ones #143

Open Szasza opened 4 years ago

Szasza commented 4 years ago

Hi @guregu ,

Thank you for maintaining the package, it is much appreciated.

I would like to ask if there has been a consideration around using interface types instead of struct types to make it possible that the various parts of the package could be mocked out using gomock and mockgen.

For example if there is a method chaining such as SomeDatabase.SomeTable(...).Scan().AllWithContext(...), currently there is no easy way to provide a mock return value for AllWithContext().

The above could be solved by having interface types used instead of struct types as Scan()-like 'factory' function return types which then can be mocked out in tests to return mock object instances.

Please don't hesitate to let me know if my use-case above is unclear, I am also happy to contribute with a PR if the above fits the general direction of the package's evolution.

guregu commented 4 years ago

Hello, thanks! Unfortunately switching everything to an interface would be a breaking change, and there's a lot of things I'd like to change if we're going to move up a major version. It could be a good idea to switch to interfaces, or maybe even use functional parameters instead of method chaining. I've stuck with this API since 2015 so it could use a brush-up for sure. I'm using this issue to consider stuff for v2: #113, input is welcomed. Maybe it would be possible to add an additional package like dynamodbiface in the aws-sdk? Just spitballing here. Let me know if you have any ideas.

In the meantime I would just mock things at a higher level (for example, data repository instead of DB level), or use integration tests against dynamodb-local or a real AWS table. You could also mock the underlying dynamodbiface but I really wouldn't suggest it as you'd have to keep up with internal stuff that might break, probably painful.

Also, kind of related: I made a fake SQL driver for mocking purposes https://github.com/guregu/mogi Maybe something like this but for dynamodbiface would be useful? Like an easy way to mock the underlying API responses. Or maybe even for dynamo instead? guregu/mogi ended up having a number of flaws that ended up biting me later, so I tend to avoid using it, but I also think it could be useful for DynamoDB if done right. Kind of went off on a tangent here, but I'm always open for ideas and stuff.

Szasza commented 4 years ago

@guregu Thank you for the quick response, it is much appreciated. Sure, it is totally understandable that things are not always simple.

As for mocking, the 'mocking via data repository' way was the one I went ahead with, by creating a repo struct + interface in the app in question and injecting the mock db connection into it, which mock db was provided by https://github.com/gusaul/go-dynamock.

Similarly to mogi, potentially a built-in mocking mechanism would be even more useful and powerful for sure however based on my understanding of dynamo that seems to be a larger chunk of work than simply providing interfaces which then can be mocked using gomock. Both fits the purpose really and it is hard to guess the future use-cases.

If there was a v2.0 branch at any point in time, I would be happy to help chipping in to the implementation. Also since there is an issue already for v2.0, please feel free to close this one at any point.

Thank you again.

kadekutama commented 4 years ago

Hi, @Szasza and @guregu . I recently created a wrapper for this library, so it can be unit-testable. Please check it out. https://github.com/kadekutama/dynamodb

Szasza commented 4 years ago

Hi @kadekutama. Awesome, thank you for sharing the info about the wrapper. I will give it a test run in the next few days to see if it simplifies the solution I used.

Thank you again.

guregu commented 4 years ago

Hey @kadekutama, that's very cool! I'll add a link to your project when I get around to improving the readme.