Merge pull request splitio#109 from splitio/development

Development

CHANGES.txt

@@ -1,3 +1,6 @@
+5.1.3 (Jan 27, 2020)
+ - Removed unnecessary Split copy made in memory.
+
 5.1.2 (Nov 28, 2019)
  - Several fixes in tests as a result of a locking & race conditions audit
  - Fixed locking issue for .Treatments() && .TreatmentsWithConfig() methods.

splitio/client/client_test.go

@@ -1145,6 +1145,17 @@ func (i *mockSegmentStorage) Get(feature string) *set.ThreadUnsafeSet {
 	return nil
 }
 
+func (i *mockSegmentStorage) SegmentContainsKey(s string, k string) (bool, error) {
+	if s != "employees" {
+		return false, fmt.Errorf("segment not found")
+	}
+
+	if k == "user1" {
+		return true, nil
+	}
+	return false, nil
+}
+
 func isInvalidImpression(client SplitClient, key string, feature string, treatment string) bool {
 	impressionsQueue := client.impressions.(storage.ImpressionStorage)
 	impressions, _ := impressionsQueue.PopN(cfg.Advanced.ImpressionsBulkSize)

splitio/engine/grammar/matchers/insegment.go

@@ -19,13 +19,11 @@ func (m *InSegmentMatcher) Match(key string, attributes map[string]interface{},
 		return false
 	}
 
-	segment := segmentStorage.Get(m.segmentName)
-	if segment == nil {
+	isInSegment, err := segmentStorage.SegmentContainsKey(m.segmentName, key)
+	if err != nil {
 		m.logger.Error(fmt.Printf("InSegmentMatcher: Segment %s not found", m.segmentName))
-		return false
 	}
-
-	return segment.Has(key)
+	return isInSegment
 }
 
 // NewInSegmentMatcher instantiates a new InSegmentMatcher

splitio/storage/interfaces.go

@@ -34,6 +34,7 @@ type SegmentStorageProducer interface {
 // SegmentStorageConsumer interface should be implemented by all structs that ofer reading segments
 type SegmentStorageConsumer interface {
 	Get(segmentName string) *set.ThreadUnsafeSet
+	SegmentContainsKey(segmentName string, key string) (bool, error)
 }
 
 // ImpressionStorageProducer interface should be impemented by structs that accept incoming impressions

splitio/storage/mutexmap/mutexmap.go

@@ -1,11 +1,11 @@
 package mutexmap
 
 import (
+	"fmt"
 	"sync"
 
 	"github.com/splitio/go-client/splitio/service/dtos"
 	"github.com/splitio/go-toolkit/datastructures/set"
-	"github.com/splitio/go-toolkit/deepcopy"
 )
 
 // ** SPLIT STORAGE **
@@ -38,8 +38,7 @@ func (m *MMSplitStorage) _get(splitName string) *dtos.SplitDTO {
 	if !exists {
 		return nil
 	}
-	c := deepcopy.Copy(item).(dtos.SplitDTO)
-	return &c
+	return &item
 }
 
 // Get retrieves a split from the MMSplitStorage
@@ -139,10 +138,7 @@ func (m *MMSplitStorage) GetAll() []dtos.SplitDTO {
 	defer m.mutex.RUnlock()
 	splitList := make([]dtos.SplitDTO, 0)
 	for _, split := range m.data {
-		splitCopy, ok := deepcopy.Copy(split).(dtos.SplitDTO)
-		if ok {
-			splitList = append(splitList, splitCopy)
-		}
+		splitList = append(splitList, split)
 	}
 	return splitList
 }
@@ -223,6 +219,17 @@ func (m *MMSegmentStorage) Get(segmentName string) *set.ThreadUnsafeSet {
 	return s
 }
 
+// SegmentContainsKey returns true if the segment contains a specific key
+func (m *MMSegmentStorage) SegmentContainsKey(segmentName string, key string) (bool, error) {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+	item, exists := m.data[segmentName]
+	if !exists {
+		return false, fmt.Errorf("segment %s not found in storage", segmentName)
+	}
+	return item.Has(key), nil
+}
+
 func (m *MMSegmentStorage) _updateTill(name string, till int64) {
 	m.tillMutex.Lock()
 	defer m.tillMutex.Unlock()

splitio/storage/mutexmap/mutexmap_test.go

@@ -242,6 +242,11 @@ func TestMMSegmentStorage(t *testing.T) {
 			if !segment.Has(element) {
 				t.Errorf("%s should be part of set number %d and isn't.", element, i)
 			}
+
+			contained, _ := segmentStorage.SegmentContainsKey(segmentName, element)
+			if !contained {
+				t.Errorf("SegmentContainsKey should return true for segment '%s' and key '%s'", segmentName, element)
+			}
 		}
 	}
 
@@ -389,3 +394,34 @@ func TestTrafficTypes(t *testing.T) {
 		t.Error("It should not exist")
 	}
 }
+
+func TestMMSplitStorageObjectLivesAfterDeletion(t *testing.T) {
+	splitStorage := NewMMSplitStorage()
+	splits := make([]dtos.SplitDTO, 10)
+	for index := 0; index < 10; index++ {
+		splits = append(splits, dtos.SplitDTO{
+			Name: fmt.Sprintf("SomeSplit_%d", index),
+			Algo: index,
+		})
+	}
+
+	splitStorage.PutMany(splits, 123)
+	someSplit0 := splitStorage.Get("SomeSplit_0")
+	splitStorage.Remove("SomeSplit_0")
+
+	if splitStorage.Get("SomeSplit_0") != nil {
+		t.Error("Should have been deleted")
+	}
+
+	if someSplit0 == nil {
+		t.Error("split0 shouldn't be nil")
+	}
+
+	if someSplit0.Name != "SomeSplit_0" {
+		t.Error("Wrong name")
+	}
+
+	if someSplit0.Algo != 0 {
+		t.Error("Wrong algo")
+	}
+}

splitio/storage/redisdb/redis.go

@@ -153,6 +153,11 @@ func (r *PrefixedRedisClient) SMembers(key string) ([]string, error) {
 	return r.client.SMembers(r.withPrefix(key)).Result()
 }
 
+// SIsMember returns a slice with all the members of a set
+func (r *PrefixedRedisClient) SIsMember(key string, item interface{}) (bool, error) {
+	return r.client.SIsMember(r.withPrefix(key), item).Result()
+}
+
 // SAdd adds new members to a set
 func (r *PrefixedRedisClient) SAdd(key string, members ...interface{}) (int64, error) {
 	return r.client.SAdd(r.withPrefix(key), members...).Result()

splitio/storage/redisdb/segments.go

@@ -41,6 +41,12 @@ func (r *RedisSegmentStorage) Get(segmentName string) *set.ThreadUnsafeSet {
 	return segment
 }
 
+// SegmentContainsKey returns true if the segment contains a specific key
+func (r *RedisSegmentStorage) SegmentContainsKey(segmentName string, key string) (bool, error) {
+	segmentKey := strings.Replace(redisSegment, "{segment}", segmentName, 1)
+	return r.client.SIsMember(segmentKey, key)
+}
+
 // Put (over)writes a segment in redis with the one passed to this function
 func (r *RedisSegmentStorage) Put(name string, segment *set.ThreadUnsafeSet, changeNumber int64) {
 	segmentKey := strings.Replace(redisSegment, "{segment}", name, 1)

splitio/storage/redisdb/storages_test.go

@@ -275,11 +275,25 @@ func TestSegmentStorage(t *testing.T) {
 		t.Error(segment1)
 	}
 
+	for _, key := range []string{"item1", "item2", "item3"} {
+		contained, _ := segmentStorage.SegmentContainsKey("segment1", key)
+		if !contained {
+			t.Errorf("SegmentContainsKey should return true for segment '%s' and key '%s'", "segment1", key)
+		}
+	}
+
 	segment2 := segmentStorage.Get("segment2")
 	if segment2 == nil || !segment2.IsEqual(set.NewSet("item4", "item5", "item6")) {
 		t.Error("Incorrect segment2")
 	}
 
+	for _, key := range []string{"item4", "item5", "item6"} {
+		contained, _ := segmentStorage.SegmentContainsKey("segment2", key)
+		if !contained {
+			t.Errorf("SegmentContainsKey should return true for segment '%s' and key '%s'", "segment2", key)
+		}
+	}
+
 	if segmentStorage.Till("segment1") != 123 || segmentStorage.Till("segment2") != 124 {
 		t.Error("Incorrect till stored")
 	}

splitio/tasks/segmentsync.go

@@ -3,13 +3,14 @@ package tasks
 import (
 	"errors"
 	"fmt"
+	"sync"
+
 	"github.com/splitio/go-client/splitio/service"
 	"github.com/splitio/go-client/splitio/storage"
 	"github.com/splitio/go-toolkit/asynctask"
 	"github.com/splitio/go-toolkit/datastructures/set"
 	"github.com/splitio/go-toolkit/logging"
 	"github.com/splitio/go-toolkit/workerpool"
-	"sync"
 )
 
 func updateSegment(

splitio/version.go

@@ -1,4 +1,4 @@
 package splitio
 
 // Version contains a string with the split sdk version
-const Version = "5.1.2"
+const Version = "5.1.3"