Add KV Maps method to load data from the KV

[arbulu89]

Implementation of the Maps method to load the KV storage value to a map. It is a generic method to load the information and use it easily. Some kind of marshalling. I creates a map because it is the most flexible way. One of the next steps would be to do the opposite way, store the data in the KV starting on a map.

This works out of the box. It doesn't matter if the all the elements in the path are created individually or in 1 shot.

I have thought to include this as consul Client method, but mockery mocks it and it makes the test different, but we can obviously do it.

This is the are key list example:

2021/04/30 16:50:20 trento/v0/environments/, 
2021/04/30 16:50:20 trento/v0/environments/env1/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/sapsystems/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/sapsystems/sys1/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/sapsystems/sys1/name, test
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/sapsystems/sys2/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land2/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land2/sapsystems/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land2/sapsystems/sys3/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land2/sapsystems/sys4/, 
2021/04/30 16:50:20 trento/v0/environments/env2/, 
2021/04/30 16:50:20 trento/v0/environments/env2/landscapes/, 
2021/04/30 16:50:20 trento/v0/environments/env2/landscapes/land3/, 
2021/04/30 16:50:20 trento/v0/environments/env2/landscapes/land3/sapsystems/, 
2021/04/30 16:50:20 trento/v0/environments/env2/landscapes/land3/sapsystems/sys5/, 

Or

2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/sapsystems/sys1/name, test
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land1/sapsystems/sys2/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land2/sapsystems/sys3/, 
2021/04/30 16:50:20 trento/v0/environments/env1/landscapes/land2/sapsystems/sys4/, 
2021/04/30 16:50:20 trento/v0/environments/env2/landscapes/land3/sapsystems/sys5/, 

Both return the same map:

2021/04/30 16:50:20 map[env1:map[landscapes:map[land1:map[sapsystems:map[sys1:map[name:test] sys2:map[]]] land2:map[sapsystems:map[sys3:map[] sys4:map[]]]]] env2:map[landscapes:map[land3:map[sapsystems:map[sys5:map[]]]]]]

This is still a working progress (the UT is a big of a nightmare, as I would need to move the kvstore.go file out of internal/consul to avoid cyclical imports.

Opinions?

[stefanotorresi]

I have thought to include this as consul Client method, but mockery mocks it and it makes the test different, but we can obviously do it.

Yeah, I would totally see this Map method as a method of our KV interface: to implement this, you will need to create a new private type kv struct {}, like we currently do for the client one, which will wrap the upstream KV object, but adds our new method to it.

I'm not quite sure why you're having cyclical imports, what packages are making the cycle? Isn't everything inside the internal/consul package here?

Other than this, though, it looks really good! 👍🏼

[arbulu89]

I'm not quite sure why you're having cyclical imports, what packages are making the cycle? Isn't everything inside the internal/consul package here?

When I want to create UT for this new Maps method, mockery creates the mocks inside internal/consul/mocks.
This mocks import the consul internal package, and if I want to import the mocks in my test of internal/consul/kvstore.go (I need to do this to mock KV List method), a cyclical import happens.

[arbulu89]

@stefanotorresi Thanks for your help. PR ready to be reviewed

[stefanotorresi]

This looks brilliant, Xabi!

Could you please update the consul import names as I mentioned in #45 (review)?

[arbulu89]

This looks brilliant, Xabi!

Could you please update the consul import names as I mentioned in #45 (review)?

Done!
I have only found some references as consul_internal in environments_test.go and hosts_test.go.
Did you see any other one?

[dirkmueller]

"Maps" is a bit hard to read for me (I read it as Google Maps not as Mappings). maybe call it "MakeMap" ?

[dirkmueller]

Maps is not very intuitive as a name for me, otherwise it looks good to me.

[arbulu89]

Maps renamed to ListMap

[stefanotorresi]

Great work, Xabi!