Skip to content

Commit

Permalink
chore: remove methods with variadic args (#117)
Browse files Browse the repository at this point in the history
* chore: remove methods with variadic args

This PR removes methods with variadic args. These overloads are
undesirable for a few reasons:
(1) They add considerable surface area to SDK.

(2) Because the arguments (cache keys/values) are variable length,
they go at the end of the method signature. This breaks the pattern
where the cache keys/values go after the cache name.

(3) Mixed support across languages. We want to add the most common
case first. In most languages, the common case is not variable length
arguments as opposed to lists.

* Relax integration test timings to work in higher latency envs.

The CI server has higher latency. We've relaxed the timings in tests
to account for this.
  • Loading branch information
malandis authored Aug 25, 2022
1 parent 6931d75 commit 801ab43
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 405 deletions.
52 changes: 7 additions & 45 deletions IncubatingIntegrationTest/DictionaryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ public async Task DictionarySetGetAsync_FieldIsByteArray_RefreshTtl()
var value = Utils.NewGuidByteArray();

await client.DictionarySetAsync(cacheName, dictionaryName, field, value, false, ttlSeconds: 1);
await Task.Delay(100);

await client.DictionarySetAsync(cacheName, dictionaryName, field, value, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -182,8 +180,6 @@ public async Task DictionarySetGetAsync_FieldIsString_RefreshTtl()
var value = Utils.NewGuidString();

await client.DictionarySetAsync(cacheName, dictionaryName, field, value, false, ttlSeconds: 1);
await Task.Delay(100);

await client.DictionarySetAsync(cacheName, dictionaryName, field, value, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -254,8 +250,6 @@ public async Task DictionarySetBatchAsync_FieldsAreByteArray_RefreshTtl()
var content = new Dictionary<byte[], byte[]>() { { field, value } };

await client.DictionarySetBatchAsync(cacheName, dictionaryName, content, false, ttlSeconds: 1);
await Task.Delay(100);

await client.DictionarySetBatchAsync(cacheName, dictionaryName, content, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -326,8 +320,6 @@ public async Task DictionarySetBatchAsync_FieldsAreStrings_RefreshTtl()
var content = new Dictionary<string, string>() { { field, value } };

await client.DictionarySetBatchAsync(cacheName, dictionaryName, content, false, ttlSeconds: 1);
await Task.Delay(100);

await client.DictionarySetBatchAsync(cacheName, dictionaryName, content, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -367,15 +359,12 @@ public async Task DictionaryGetBatchAsync_FieldsAreByteArray_HappyPath()
await client.DictionarySetAsync(cacheName, dictionaryName, field1, value1, false, 10);
await client.DictionarySetAsync(cacheName, dictionaryName, field2, value2, false, 10);

var response1 = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, field1, field2, field3);
var response2 = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, new byte[][] { field1, field2, field3 });
var response = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, new byte[][] { field1, field2, field3 });

var status = new CacheGetStatus[] { CacheGetStatus.HIT, CacheGetStatus.HIT, CacheGetStatus.MISS };
var values = new byte[]?[] { value1, value2, null };
Assert.Equal(status, response1.Status);
Assert.Equal(status, response2.Status);
Assert.Equal(values, response1.ByteArrays);
Assert.Equal(values, response2.ByteArrays);
Assert.Equal(status, response.Status);
Assert.Equal(values, response.ByteArrays);
}

[Fact]
Expand All @@ -386,7 +375,7 @@ public async Task DictionaryGetBatchAsync_DictionaryMissing_HappyPath()
var field2 = Utils.NewGuidByteArray();
var field3 = Utils.NewGuidByteArray();

var response = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, field1, field2, field3);
var response = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, new byte[][] { field1, field2, field3 });

var status = new CacheGetStatus[] { CacheGetStatus.MISS, CacheGetStatus.MISS, CacheGetStatus.MISS };
var byteArrays = new byte[]?[] { null, null, null };
Expand Down Expand Up @@ -427,15 +416,12 @@ public async Task DictionaryGetBatchAsync_FieldsAreStrings_HappyPath()
await client.DictionarySetAsync(cacheName, dictionaryName, field1, value1, false, 10);
await client.DictionarySetAsync(cacheName, dictionaryName, field2, value2, false, 10);

var response1 = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, field1, field2, field3);
var response2 = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, new string[] { field1, field2, field3 });
var response = await client.DictionaryGetBatchAsync(cacheName, dictionaryName, new string[] { field1, field2, field3 });

var status = new CacheGetStatus[] { CacheGetStatus.HIT, CacheGetStatus.HIT, CacheGetStatus.MISS };
var values = new string?[] { value1, value2, null };
Assert.Equal(status, response1.Status);
Assert.Equal(status, response2.Status);
Assert.Equal(values, response1.Strings());
Assert.Equal(values, response2.Strings());
Assert.Equal(status, response.Status);
Assert.Equal(values, response.Strings());
}

[Theory]
Expand Down Expand Up @@ -624,19 +610,7 @@ public async Task DictionaryRemoveFieldsAsync_ByteArrayParams_HappyPath()
var fields = new byte[][] { Utils.NewGuidByteArray(), Utils.NewGuidByteArray() };
var otherField = Utils.NewGuidByteArray();

// Test variadic args
await client.DictionarySetAsync(cacheName, dictionaryName, fields[0], Utils.NewGuidByteArray(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, fields[1], Utils.NewGuidByteArray(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, otherField, Utils.NewGuidByteArray(), false);


await client.DictionaryRemoveFieldsAsync(cacheName, dictionaryName, fields[0], fields[1]);
Assert.Equal(CacheGetStatus.MISS, (await client.DictionaryGetAsync(cacheName, dictionaryName, fields[0])).Status);
Assert.Equal(CacheGetStatus.MISS, (await client.DictionaryGetAsync(cacheName, dictionaryName, fields[1])).Status);
Assert.Equal(CacheGetStatus.HIT, (await client.DictionaryGetAsync(cacheName, dictionaryName, otherField)).Status);

// Test enumerable
dictionaryName = Utils.NewGuidString();
await client.DictionarySetAsync(cacheName, dictionaryName, fields[0], Utils.NewGuidByteArray(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, fields[1], Utils.NewGuidByteArray(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, otherField, Utils.NewGuidByteArray(), false);
Expand Down Expand Up @@ -672,19 +646,7 @@ public async Task DictionaryRemoveFieldsAsync_StringParams_HappyPath()
var fields = new string[] { Utils.NewGuidString(), Utils.NewGuidString() };
var otherField = Utils.NewGuidString();

// Test variadic args
await client.DictionarySetAsync(cacheName, dictionaryName, fields[0], Utils.NewGuidString(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, fields[1], Utils.NewGuidString(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, otherField, Utils.NewGuidString(), false);


await client.DictionaryRemoveFieldsAsync(cacheName, dictionaryName, fields[0], fields[1]);
Assert.Equal(CacheGetStatus.MISS, (await client.DictionaryGetAsync(cacheName, dictionaryName, fields[0])).Status);
Assert.Equal(CacheGetStatus.MISS, (await client.DictionaryGetAsync(cacheName, dictionaryName, fields[1])).Status);
Assert.Equal(CacheGetStatus.HIT, (await client.DictionaryGetAsync(cacheName, dictionaryName, otherField)).Status);

// Test enumerable
dictionaryName = Utils.NewGuidString();
await client.DictionarySetAsync(cacheName, dictionaryName, fields[0], Utils.NewGuidString(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, fields[1], Utils.NewGuidString(), false);
await client.DictionarySetAsync(cacheName, dictionaryName, otherField, Utils.NewGuidString(), false);
Expand Down
8 changes: 0 additions & 8 deletions IncubatingIntegrationTest/ListTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ public async Task ListPushFrontFetch_ValueIsByteArray_RefreshTtl()
var value = Utils.NewGuidByteArray();

await client.ListPushFrontAsync(cacheName, listName, value, false, ttlSeconds: 1);
await Task.Delay(100);

await client.ListPushFrontAsync(cacheName, listName, value, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -133,8 +131,6 @@ public async Task ListPushFrontFetch_ValueIsString_RefreshTtl()
var value = Utils.NewGuidString();

await client.ListPushFrontAsync(cacheName, listName, value, false, ttlSeconds: 1);
await Task.Delay(100);

await client.ListPushFrontAsync(cacheName, listName, value, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -200,8 +196,6 @@ public async Task ListPushBackFetch_ValueIsByteArray_RefreshTtl()
var value = Utils.NewGuidByteArray();

await client.ListPushBackAsync(cacheName, listName, value, false, ttlSeconds: 1);
await Task.Delay(100);

await client.ListPushBackAsync(cacheName, listName, value, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -267,8 +261,6 @@ public async Task ListPushBackFetch_ValueIsString_RefreshTtl()
var value = Utils.NewGuidString();

await client.ListPushBackAsync(cacheName, listName, value, false, ttlSeconds: 1);
await Task.Delay(100);

await client.ListPushBackAsync(cacheName, listName, value, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down
132 changes: 2 additions & 130 deletions IncubatingIntegrationTest/SetTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ public async Task SetAddFetch_ElementIsByteArray_RefreshTtl()
var element = Utils.NewGuidByteArray();

await client.SetAddAsync(cacheName, setName, element, false, ttlSeconds: 1);
await Task.Delay(100);

await client.SetAddAsync(cacheName, setName, element, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -115,8 +113,6 @@ public async Task SetAddFetch_ElementIsString_RefreshTtl()
var element = Utils.NewGuidString();

await client.SetAddAsync(cacheName, setName, element, false, ttlSeconds: 1);
await Task.Delay(100);

await client.SetAddAsync(cacheName, setName, element, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand All @@ -136,66 +132,6 @@ public async Task SetAddBatchAsync_NullChecksByteArray_ThrowsException()

set.Add(null!);
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(cacheName, setName, set, false));

// Variadic args
var byteArray = Utils.NewGuidByteArray();
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(null!, setName, false, ttlSeconds: null, byteArray));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(cacheName, null!, false, ttlSeconds: null, byteArray));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: null, (byte[])null!));
}

[Fact]
public async Task SetAddBatchAsync_ElementsAreByteArrayParams_HappyPath()
{
var setName = Utils.NewGuidString();
var element1 = Utils.NewGuidByteArray();
var element2 = Utils.NewGuidByteArray();

await client.SetAddBatchAsync(cacheName, setName, false, 10, element1, element2);

var fetchResponse = await client.SetFetchAsync(cacheName, setName);
Assert.Equal(CacheGetStatus.HIT, fetchResponse.Status);

var set = fetchResponse.ByteArraySet;
Assert.Equal(2, set!.Count);
Assert.Contains(element1, set);
Assert.Contains(element2, set);
}

[Fact]
public async Task SetAddBatchAsync_ElementsAreByteArrayParams_NoRefreshTtl()
{
var setName = Utils.NewGuidString();
var element = Utils.NewGuidByteArray();

await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: 5, element);
await Task.Delay(100);

await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: 10, element);
await Task.Delay(4900);

var response = await client.SetFetchAsync(cacheName, setName);
Assert.Equal(CacheGetStatus.MISS, response.Status);
}

[Fact]
public async Task SetAddBatchAsync_ElementsAreByteArrayParams_RefreshTtl()
{
var setName = Utils.NewGuidString();
var element = Utils.NewGuidByteArray();

await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: 1, element);
await Task.Delay(100);

await client.SetAddBatchAsync(cacheName, setName, true, ttlSeconds: 10, element);
await Task.Delay(1000);

var response = await client.SetFetchAsync(cacheName, setName);
Assert.Equal(CacheGetStatus.HIT, response.Status);

var set = response.ByteArraySet;
Assert.Single(set);
Assert.Contains(element, set);
}

[Fact]
Expand Down Expand Up @@ -242,8 +178,6 @@ public async Task SetAddBatchAsync_ElementsAreByteArrayEnumerable_RefreshTtl()
var content = new List<byte[]>() { element };

await client.SetAddBatchAsync(cacheName, setName, content, false, ttlSeconds: 1);
await Task.Delay(100);

await client.SetAddBatchAsync(cacheName, setName, content, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand All @@ -266,66 +200,6 @@ public async Task SetAddBatchAsync_NullChecksString_ThrowsException()

set.Add(null!);
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(cacheName, setName, set, false));

// Variadic args
var str = Utils.NewGuidString();
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(null!, setName, false, ttlSeconds: null, str));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(cacheName, null!, false, ttlSeconds: null, str));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: null, (string)null!));
}

[Fact]
public async Task SetAddBatchAsync_ElementsAreStringParams_HappyPath()
{
var setName = Utils.NewGuidString();
var element1 = Utils.NewGuidString();
var element2 = Utils.NewGuidString();

await client.SetAddBatchAsync(cacheName, setName, false, 10, element1, element2);

var fetchResponse = await client.SetFetchAsync(cacheName, setName);
Assert.Equal(CacheGetStatus.HIT, fetchResponse.Status);

var set = fetchResponse.StringSet();
Assert.Equal(2, set!.Count);
Assert.Contains(element1, set);
Assert.Contains(element2, set);
}

[Fact]
public async Task SetAddBatchAsync_ElementsAreStringParams_NoRefreshTtl()
{
var setName = Utils.NewGuidString();
var element = Utils.NewGuidString();

await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: 5, element);
await Task.Delay(100);

await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: 10, element);
await Task.Delay(4900);

var response = await client.SetFetchAsync(cacheName, setName);
Assert.Equal(CacheGetStatus.MISS, response.Status);
}

[Fact]
public async Task SetAddBatchAsync_ElementsAreStringParams_RefreshTtl()
{
var setName = Utils.NewGuidString();
var element = Utils.NewGuidString();

await client.SetAddBatchAsync(cacheName, setName, false, ttlSeconds: 1, element);
await Task.Delay(100);

await client.SetAddBatchAsync(cacheName, setName, true, ttlSeconds: 10, element);
await Task.Delay(1000);

var response = await client.SetFetchAsync(cacheName, setName);
Assert.Equal(CacheGetStatus.HIT, response.Status);

var set = response.StringSet();
Assert.Single(set);
Assert.Contains(element, set);
}

[Fact]
Expand Down Expand Up @@ -372,8 +246,6 @@ public async Task SetAddBatchAsync_ElementsAreStringEnumerable_RefreshTtl()
var content = new List<string>() { element };

await client.SetAddBatchAsync(cacheName, setName, content, false, ttlSeconds: 1);
await Task.Delay(100);

await client.SetAddBatchAsync(cacheName, setName, content, true, ttlSeconds: 10);
await Task.Delay(1000);

Expand Down Expand Up @@ -407,7 +279,7 @@ public async Task SetFetchAsync_Missing_HappyPath()
public async Task SetFetchAsync_UsesCachedByteArraySet_HappyPath()
{
var setName = Utils.NewGuidString();
await client.SetAddBatchAsync(cacheName, setName, false, null, Utils.NewGuidString(), Utils.NewGuidString());
await client.SetAddBatchAsync(cacheName, setName, new string[] { Utils.NewGuidString(), Utils.NewGuidString() }, false);
var response = await client.SetFetchAsync(cacheName, setName);

var set1 = response.ByteArraySet;
Expand All @@ -419,7 +291,7 @@ public async Task SetFetchAsync_UsesCachedByteArraySet_HappyPath()
public async Task SetFetchAsync_UsesCachedStringSet_HappyPath()
{
var setName = Utils.NewGuidString();
await client.SetAddBatchAsync(cacheName, setName, false, null, Utils.NewGuidString(), Utils.NewGuidString());
await client.SetAddBatchAsync(cacheName, setName, new string[] { Utils.NewGuidString(), Utils.NewGuidString() }, false);
var response = await client.SetFetchAsync(cacheName, setName);

var set1 = response.StringSet();
Expand Down
16 changes: 0 additions & 16 deletions Momento/ISimpleCacheClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,22 +111,6 @@ public interface ISimpleCacheClient : IDisposable
/// <returns>Task object representing the statuses of the get operation and the associated values.</returns>
public Task<CacheGetBatchResponse> GetBatchAsync(string cacheName, IEnumerable<string> keys);

/// <summary>
/// Gets multiple values from the cache.
/// </summary>
/// <param name="cacheName">Name of the cache to perform the lookup in.</param>
/// <param name="keys">The keys to get.</param>
/// <returns>Task object representing the statuses of the get operation and the associated values.</returns>
public Task<CacheGetBatchResponse> GetBatchAsync(string cacheName, params byte[][] keys);

/// <summary>
/// Gets multiple values from the cache.
/// </summary>
/// <param name="cacheName">Name of the cache to perform the lookup in.</param>
/// <param name="keys">The keys to get.</param>
/// <returns>Task object representing the statuses of the get operation and the associated values.</returns>
public Task<CacheGetBatchResponse> GetBatchAsync(string cacheName, params string[] keys);

/// <summary>
/// Sets multiple items in the cache. Overwrites existing items.
/// </summary>
Expand Down
Loading

0 comments on commit 801ab43

Please sign in to comment.