cnabio / cnab-go

A Go implementation of CNAB Core 1.0
MIT License
69 stars 37 forks source link

Improve autoclose logic #211

Closed carolynvs closed 4 years ago

carolynvs commented 4 years ago

Only autoclose the crud store in a function when that function is the one that called Connect. This removes the burden from functions that call other functions that use a crud store from having to keep track of whether or not AutoClose is enabled.

This isn't used much currently, it improves ReadAll, but it will be much more useful in the new functions incoming for supporting the claims spec update.

carolynvs commented 4 years ago

Are there any new variations or updates to introduce to the unit tests?

The existing code had regression tests around when open and close are called and how often, and in cases where the caller had already opened the connection (e.g. in ReadAll it opens the connection, then List and Reads should not try to open/close).

https://github.com/cnabio/cnab-go/blob/682f211c7dfe5d93cb880d0c4aa0f958a841b563/utils/crud/backingstore_test.go#L174-L211

This PR didn't change didn't change any existing behavior, just made sure that for more interesting functions than ReadAll that it would be easy for the caller to manage safely. So I think those regression tests should still be sufficient.