From bad10a4ce0d07204a3d556140f7b4fa0817df083 Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Sat, 16 Dec 2023 02:47:37 +0530 Subject: [PATCH] address review comments --- src/internal/common/keys/keys.go | 31 +++++++++ .../keyset_test.go => keys/keys_test.go} | 64 ++++++++++++++++--- src/internal/common/maps/keyset.go | 21 ------ src/internal/common/maps/maps.go | 11 ---- src/internal/common/maps/maps_test.go | 62 ------------------ src/pkg/services/m365/api/lists.go | 12 ++-- 6 files changed, 91 insertions(+), 110 deletions(-) create mode 100644 src/internal/common/keys/keys.go rename src/internal/common/{maps/keyset_test.go => keys/keys_test.go} (50%) delete mode 100644 src/internal/common/maps/keyset.go delete mode 100644 src/internal/common/maps/maps.go delete mode 100644 src/internal/common/maps/maps_test.go diff --git a/src/internal/common/keys/keys.go b/src/internal/common/keys/keys.go new file mode 100644 index 0000000000..07d0cf4444 --- /dev/null +++ b/src/internal/common/keys/keys.go @@ -0,0 +1,31 @@ +package keys + +type Set map[string]struct{} + +func (ks Set) HasKey(key string) bool { + if _, ok := ks[key]; ok { + return true + } + + return false +} + +func (ks Set) Keys() []string { + sliceKeys := make([]string, 0) + + for k := range ks { + sliceKeys = append(sliceKeys, k) + } + + return sliceKeys +} + +func HasKeys(data map[string]any, keys ...string) bool { + for _, k := range keys { + if _, ok := data[k]; !ok { + return false + } + } + + return true +} diff --git a/src/internal/common/maps/keyset_test.go b/src/internal/common/keys/keys_test.go similarity index 50% rename from src/internal/common/maps/keyset_test.go rename to src/internal/common/keys/keys_test.go index f0a582f211..41e6de6d97 100644 --- a/src/internal/common/maps/keyset_test.go +++ b/src/internal/common/keys/keys_test.go @@ -1,4 +1,4 @@ -package maps +package keys import ( "testing" @@ -17,28 +17,28 @@ func TestKeySetTestSuite(t *testing.T) { suite.Run(t, &KeySetTestSuite{Suite: tester.NewUnitSuite(t)}) } -func (suite *KeySetTestSuite) Test_HasKey() { +func (suite *KeySetTestSuite) TestHasKey() { tests := []struct { name string - keySet KeySet + keySet Set key string expect assert.BoolAssertionFunc }{ { name: "key exists in the set", - keySet: KeySet{"key1": {}, "key2": {}}, + keySet: Set{"key1": {}, "key2": {}}, key: "key1", expect: assert.True, }, { name: "key does not exist in the set", - keySet: KeySet{"key1": {}, "key2": {}}, + keySet: Set{"key1": {}, "key2": {}}, key: "nonexistent", expect: assert.False, }, { name: "empty set", - keySet: KeySet{}, + keySet: Set{}, key: "key", expect: assert.False, }, @@ -51,20 +51,20 @@ func (suite *KeySetTestSuite) Test_HasKey() { } } -func (suite *KeySetTestSuite) Test_Keys() { +func (suite *KeySetTestSuite) TestKeys() { tests := []struct { name string - keySet KeySet + keySet Set expect assert.ValueAssertionFunc }{ { name: "non-empty set", - keySet: KeySet{"key1": {}, "key2": {}}, + keySet: Set{"key1": {}, "key2": {}}, expect: assert.NotEmpty, }, { name: "empty set", - keySet: KeySet{}, + keySet: Set{}, expect: assert.Empty, }, } @@ -76,3 +76,47 @@ func (suite *KeySetTestSuite) Test_Keys() { }) } } + +func (suite *KeySetTestSuite) TestHasKeys() { + tests := []struct { + name string + data map[string]any + keys []string + expect assert.BoolAssertionFunc + }{ + { + name: "has all keys", + data: map[string]any{ + "key1": "data1", + "key2": 2, + "key3": struct{}{}, + }, + keys: []string{"key1", "key2", "key3"}, + expect: assert.True, + }, + { + name: "has some keys", + data: map[string]any{ + "key1": "data1", + "key2": 2, + }, + keys: []string{"key1", "key2", "key3"}, + expect: assert.False, + }, + { + name: "has no key", + data: map[string]any{ + "key1": "data1", + "key2": 2, + }, + keys: []string{"key4", "key5", "key6"}, + expect: assert.False, + }, + } + + for _, test := range tests { + suite.Run(test.name, func() { + test.expect(suite.T(), HasKeys(test.data, test.keys...)) + }) + } +} diff --git a/src/internal/common/maps/keyset.go b/src/internal/common/maps/keyset.go deleted file mode 100644 index 8502b93507..0000000000 --- a/src/internal/common/maps/keyset.go +++ /dev/null @@ -1,21 +0,0 @@ -package maps - -type KeySet map[string]struct{} - -func (ks KeySet) HasKey(key string) bool { - if _, ok := ks[key]; ok { - return true - } - - return false -} - -func (ks KeySet) Keys() []string { - sliceKeys := make([]string, 0) - - for k := range ks { - sliceKeys = append(sliceKeys, k) - } - - return sliceKeys -} diff --git a/src/internal/common/maps/maps.go b/src/internal/common/maps/maps.go deleted file mode 100644 index d566f90d9f..0000000000 --- a/src/internal/common/maps/maps.go +++ /dev/null @@ -1,11 +0,0 @@ -package maps - -func HasKeys(data map[string]any, keys ...string) bool { - for _, k := range keys { - if _, ok := data[k]; !ok { - return false - } - } - - return true -} diff --git a/src/internal/common/maps/maps_test.go b/src/internal/common/maps/maps_test.go deleted file mode 100644 index cdc2481269..0000000000 --- a/src/internal/common/maps/maps_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package maps - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/suite" - - "github.com/alcionai/corso/src/internal/tester" -) - -type MapsUnitTestSuite struct { - tester.Suite -} - -func TestMapsUnitTestSuite(t *testing.T) { - suite.Run(t, &MapsUnitTestSuite{Suite: tester.NewUnitSuite(t)}) -} - -func (suite *MapsUnitTestSuite) Test_HasKeys() { - tests := []struct { - name string - data map[string]any - keys []string - expect assert.BoolAssertionFunc - }{ - { - name: "has all keys", - data: map[string]any{ - "key1": "data1", - "key2": 2, - "key3": struct{}{}, - }, - keys: []string{"key1", "key2", "key3"}, - expect: assert.True, - }, - { - name: "has some keys", - data: map[string]any{ - "key1": "data1", - "key2": 2, - }, - keys: []string{"key1", "key2", "key3"}, - expect: assert.False, - }, - { - name: "has no key", - data: map[string]any{ - "key1": "data1", - "key2": 2, - }, - keys: []string{"key4", "key5", "key6"}, - expect: assert.False, - }, - } - - for _, test := range tests { - suite.Run(test.name, func() { - test.expect(suite.T(), HasKeys(test.data, test.keys...)) - }) - } -} diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index ed08a16bbd..1b9657312f 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -7,7 +7,7 @@ import ( "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" - "github.com/alcionai/corso/src/internal/common/maps" + "github.com/alcionai/corso/src/internal/common/keys" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) @@ -61,13 +61,13 @@ var readOnlyAddressFieldNames = []string{ DispNameFieldName, } -var legacyColumns = maps.KeySet{ +var legacyColumns = keys.Set{ AttachmentsColumnName: {}, EditColumnName: {}, ContentTypeColumnDisplayName: {}, } -var readOnlyFieldNames = maps.KeySet{ +var readOnlyFieldNames = keys.Set{ AttachmentsColumnName: {}, EditColumnName: {}, ContentTypeColumnName: {}, @@ -533,17 +533,17 @@ func retainPrimaryAddressField(additionalData map[string]any) { } func hasAddressFields(additionalData map[string]any) bool { - if !maps.HasKeys(additionalData, readOnlyAddressFieldNames...) { + if !keys.HasKeys(additionalData, readOnlyAddressFieldNames...) { return false } for _, value := range additionalData { nestedFields, ok := value.(map[string]any) - if !ok || maps.HasKeys(nestedFields, GeoLocFieldName) { + if !ok || keys.HasKeys(nestedFields, GeoLocFieldName) { continue } - if maps.HasKeys(nestedFields, addressFieldNames...) { + if keys.HasKeys(nestedFields, addressFieldNames...) { return true } }