feat: support HSET in redis

[debanjan97]

Description

This PR augments the capability of the redisManager
to support HSET as one of the operations.
More on HSET here.
The customdestinationmanager checks the format of the incoming event
and determines the type of operation required for writing into the key-value store.
If the incoming event is in the format of

{
  "hash": "..." ,
  "key": "...",
  "value": "..."
}

customdestinationmanager decides to use HSET.

More on why this change is required is here

Linear Ticket

https://linear.app/rudderstack/issue/PRO-633/server-add-support-for-hset

Security

• The code changed/added as part of this pull request won't create any security issues with how the software is being used.

[linear]

PRO-633 Server: Add support for HSET

Details: https://www.notion.so/rudderstacks/Profiles-API-Redis-destination-tech-doc-9abf179ceac145c6b5629d0c879afe2a?pvs=4#d8d217804ee64bea89c0b5e7a40d4383

[codecov]

Codecov Report

Patch coverage: 70.83% and project coverage change: +0.12% 🎉

Comparison is base (3dff32d) 68.94% compared to head (9a9cf61) 69.07%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3768      +/-   ##
==========================================
+ Coverage   68.94%   69.07%   +0.12%     
==========================================
  Files         349      349              
  Lines       51872    51894      +22     
==========================================
+ Hits        35765    35844      +79     
+ Misses      13801    13745      -56     
+ Partials     2306     2305       -1     

Files Changed	Coverage Δ
services/kvstoremanager/redis.go	42.57% <0.00%> (+11.72%)	⬆️
...stomdestinationmanager/customdestinationmanager.go	59.25% <100.00%> (+7.56%)	⬆️
services/kvstoremanager/kvstoremanager.go	100.00% <100.00%> (+38.09%)	⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[achettyiitr]

[Optional] We can remove the customdestinationmanager_suite_test.go, since there are no ginkgo-based tests present within the package.

[achettyiitr]

[Optional] Maybe we can remove duplicate code and compact these tests?

Suggested change

	func TestKVManagerHSETInvocation(t *testing.T) {
	initCustomerManager()
	customManager := New("REDIS", Opts{}).(*CustomManagerT)
	someDestination := backendconfig.DestinationT{
	ID: "someDestinationID1",
	DestinationDefinition: backendconfig.DestinationDefinitionT{
	Name: "REDIS",
	},
	}
	err := customManager.onNewDestination(someDestination)
	assert.Nil(t, err)
	event := json.RawMessage(`{
	"message": {
	"key": "someKey",
	"value" : "someValue",
	"hash": "someHash"
	}
	}
	`)
	ctrl := gomock.NewController(t)
	mockKVStoreManager := mock_kvstoremanager.NewMockKVStoreManager(ctrl)
	mockKVStoreManager.EXPECT().HSet("someHash", "someKey", "someValue").Times(1)
	mockKVStoreManager.EXPECT().StatusCode(nil).Times(1)
	customManager.send(event, mockKVStoreManager, someDestination.Config)
	}
	func TestKVManagerHMSETInvocation(t *testing.T) {
	initCustomerManager()
	customManager := New("REDIS", Opts{}).(*CustomManagerT)
	someDestination := backendconfig.DestinationT{
	ID: "someDestinationID1",
	DestinationDefinition: backendconfig.DestinationDefinitionT{
	Name: "REDIS",
	},
	}
	err := customManager.onNewDestination(someDestination)
	assert.Nil(t, err)
	event := json.RawMessage(`{
	"message": {
	"key": "someKey",
	"fields" : {
	"field1": "value1",
	"field2": "value2"
	}
	}
	}
	`)
	ctrl := gomock.NewController(t)
	mockKVStoreManager := mock_kvstoremanager.NewMockKVStoreManager(ctrl)
	mockKVStoreManager.EXPECT().HMSet("someKey", map[string]interface{}{
	"field1": "value1",
	"field2": "value2",
	}).Times(1)
	mockKVStoreManager.EXPECT().StatusCode(nil).Times(1)
	customManager.send(event, mockKVStoreManager, someDestination.Config)
	}
	func TestKVManagerInvocation(t *testing.T) {
	initCustomerManager()
	customManager := New("REDIS", Opts{}).(*CustomManagerT)
	someDestination := backendconfig.DestinationT{
	ID: "someDestinationID1",
	DestinationDefinition: backendconfig.DestinationDefinitionT{
	Name: "REDIS",
	},
	}
	err := customManager.onNewDestination(someDestination)
	assert.Nil(t, err)
	ctrl := gomock.NewController(t)
	defer ctrl.Finish()
	t.Run("HSET", func(t *testing.T) {
	event := json.RawMessage(`{
	"message": {
	"key": "someKey",
	"value" : "someValue",
	"hash": "someHash"
	}
	}
	`)
	mockKVStoreManager := mock_kvstoremanager.NewMockKVStoreManager(ctrl)
	mockKVStoreManager.EXPECT().HSet("someHash", "someKey", "someValue").Times(1)
	mockKVStoreManager.EXPECT().StatusCode(nil).Times(1)
	customManager.send(event, mockKVStoreManager, someDestination.Config)
	})
	t.Run("HMSET", func(t *testing.T) {
	event := json.RawMessage(`{
	"message": {
	"key": "someKey",
	"fields" : {
	"field1": "value1",
	"field2": "value2"
	}
	}
	}
	`)
	mockKVStoreManager := mock_kvstoremanager.NewMockKVStoreManager(ctrl)
	mockKVStoreManager.EXPECT().HMSet("someKey", map[string]interface{}{
	"field1": "value1",
	"field2": "value2",
	}).Times(1)
	mockKVStoreManager.EXPECT().StatusCode(nil).Times(1)
	customManager.send(event, mockKVStoreManager, someDestination.Config)
	})
	}

[debanjan97]

addressed

[lvrach]

I would avoid the extensive and repeated usage of gjson.

Using a go struct

type HSetEvent struct {
   Hash string `json:"<hash_path>"`
   Key string `json:"<key_path>"`
   Value string `json:"<value_path>"`
}

It would be safer and faster (assuming fastjson library is used)

[debanjan97]

I am not sure if I understand faster (assuming fastjson library is used) completely.

I can use fastjson's Exists but that doesn't require a go struct to unmarshal to.
Unless we somehow use fastjson while unmarshalling, using CustomUnmarshal, or I might be completely missing the point here.

[a-rampalli]

lgtm