Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #7804: Remove HasChanged #7908

Merged
merged 3 commits into from
Oct 4, 2019
Merged

Fix #7804: Remove HasChanged #7908

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b812198
Fix #7804: Remove HasChanged
AlexanderSher Oct 3, 2019
34f35f9
Remove unused field
AlexanderSher Oct 3, 2019
369ca88
Merge remote-tracking branch 'Azure/master' into Fix7804
AlexanderSher Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 0 additions & 80 deletions sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -851,86 +851,6 @@ private Request CreateGetRevisionsRequest(SettingSelector selector, string pageL
return request;
}

/// <summary>
/// </summary>
/// <param name="setting"></param>
/// <param name="cancellationToken"></param>
/// <returns></returns>
public virtual async Task<Response<bool>> HasChangedAsync(ConfigurationSetting setting, CancellationToken cancellationToken = default)
{
using DiagnosticScope scope = _pipeline.Diagnostics.CreateScope("Azure.Data.AppConfiguration.ConfigurationClient.HasChanged");
scope.AddAttribute("key", setting.Key);
scope.Start();

try
{
using Request request = CreateHeadRequest(setting.Key, setting.Label);
Response response = await _pipeline.SendRequestAsync(request, cancellationToken).ConfigureAwait(false);

switch (response.Status)
{
case 200:
case 404:
string etag = default;
if (response.Headers.TryGetValue(ETag, out etag))
{
etag = etag.Trim('\"');
}

return Response.FromValue(response, setting.ETag != new ETag(etag));

default:
throw await response.CreateRequestFailedExceptionAsync().ConfigureAwait(false);
};
}
catch (Exception e)
{
scope.Failed(e);
throw;
}
}

/// <summary>
/// </summary>
/// <param name="setting"></param>
/// <param name="cancellationToken"></param>
/// <returns></returns>
public virtual Response<bool> HasChanged(ConfigurationSetting setting, CancellationToken cancellationToken = default)
{
using DiagnosticScope scope = _pipeline.Diagnostics.CreateScope("Azure.Data.AppConfiguration.ConfigurationClient.HasChanged");
scope.AddAttribute(nameof(setting.Key), setting.Key);
scope.Start();

try
{
using (Request request = CreateHeadRequest(setting.Key, setting.Label))
{
Response response = _pipeline.SendRequest(request, cancellationToken);

switch (response.Status)
{
case 200:
case 404:
string etag = default;
if (response.Headers.TryGetValue(ETag, out etag))
{
etag = etag.Trim('\"');
}

return Response.FromValue(response, setting.ETag != new ETag(etag));

default:
throw response.CreateRequestFailedException();
}
}
}
catch (Exception e)
{
scope.Failed(e);
throw;
}
}

private Request CreateHeadRequest(string key, string label)
{
if (string.IsNullOrEmpty(key))
Expand Down
1 change: 0 additions & 1 deletion sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient_private.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ public partial class ConfigurationClient
private const string KeyQueryFilter = "key";
private const string LabelQueryFilter = "label";
private const string FieldsQueryFilter = "$select";
private const string ETag = "ETag";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@annelo-msft is this something that will be used in the conditional get change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. No, that's only used in the HasChanged logic so it can go.


private static readonly char[] s_reservedCharacters = new char[] { ',', '\\' };

Expand Down
97 changes: 0 additions & 97 deletions sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationLiveTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,103 +869,6 @@ public async Task GetBatchSettingWithAllFields()
}
}

[Test]
public async Task HasChangedNoValueNotFound()
{
ConfigurationClient service = GetClient();
ConfigurationSetting setting0 = CreateSetting();

bool hasChanged = await service.HasChangedAsync(setting0);

Assert.IsFalse(hasChanged);
}

[Test]
public async Task HasChangedNoValueNewValue()
{
ConfigurationClient service = GetClient();
ConfigurationSetting setting0 = CreateSetting();

try
{
ConfigurationSetting setting1 = await service.SetAsync(setting0);

// Test
bool hasChanged = await service.HasChangedAsync(setting0);

Assert.IsTrue(hasChanged);
}
finally
{
await service.DeleteAsync(setting0.Key, setting0.Label);
}
}

[Test]
public async Task HasChangedValuesMatch()
{
ConfigurationClient service = GetClient();
ConfigurationSetting setting0 = CreateSetting();

try
{
ConfigurationSetting setting1 = await service.SetAsync(setting0);

// Test
bool hasChanged = await service.HasChangedAsync(setting1);

Assert.IsFalse(hasChanged);
}
finally
{
await service.DeleteAsync(setting0.Key, setting0.Label);
}
}

[Test]
public async Task HasChangedValuesDontMatch()
{
ConfigurationClient service = GetClient();
ConfigurationSetting setting0 = CreateSetting();

try
{
ConfigurationSetting setting1 = await service.SetAsync(setting0);
setting0.Value = "test_value2";
ConfigurationSetting setting2 = await service.SetAsync(setting0);

// Test
bool hasChanged = await service.HasChangedAsync(setting1);

Assert.IsTrue(hasChanged);
}
finally
{
await service.DeleteAsync(setting0.Key, setting0.Label);
}
}

[Test]
public async Task HasChangedValueNotFound()
{
ConfigurationClient service = GetClient();
ConfigurationSetting setting0 = CreateSetting();

try
{
ConfigurationSetting setting1 = await service.SetAsync(setting0);
var response = await service.DeleteAsync(setting1.Key, setting1.Label);

// Test
bool hasChanged = await service.HasChangedAsync(setting1);

Assert.IsTrue(hasChanged);
}
finally
{
await service.DeleteAsync(setting0.Key, setting0.Label);
}
}

[Test]
public async Task SetReadOnlyOnSetting()
Expand Down
125 changes: 0 additions & 125 deletions sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationMockTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,131 +636,6 @@ public async Task AuthorizationHeaderFormat()
Assert.True(Regex.IsMatch(authHeader, expectedSyntax));
}

[Test]
public async Task HasChangedNoValueNotFound()
{
var response = new MockResponse(404);

var mockTransport = new MockTransport(response);
ConfigurationClient service = CreateTestService(mockTransport);

bool hasChanged = await service.HasChangedAsync(s_testSetting);

MockRequest request = mockTransport.SingleRequest;

AssertRequestCommon(request);
Assert.AreEqual(RequestMethod.Head, request.Method);
Assert.AreEqual($"https://contoso.appconfig.io/kv/test_key?label=test_label&api-version={s_version}", request.Uri.ToString());
Assert.IsFalse(response.Headers.TryGetValue("ETag", out string etag));
Assert.IsFalse(hasChanged);
}

[Test]
public async Task HasChangedNoValueNewValue()
{
var response = new MockResponse(200);

var unversionedSetting = s_testSetting.Clone();

string version = Guid.NewGuid().ToString();
var versionedSetting = s_testSetting.Clone();
versionedSetting.ETag = new ETag(version);

response.SetContent(SerializationHelpers.Serialize(versionedSetting, SerializeSetting));
response.AddHeader(new HttpHeader("ETag", versionedSetting.ETag.ToString()));

var mockTransport = new MockTransport(response);
ConfigurationClient service = CreateTestService(mockTransport);

bool hasChanged = await service.HasChangedAsync(unversionedSetting);
MockRequest request = mockTransport.SingleRequest;

AssertRequestCommon(request);
Assert.AreEqual(RequestMethod.Head, request.Method);
Assert.AreEqual($"https://contoso.appconfig.io/kv/test_key?label=test_label&api-version={s_version}", request.Uri.ToString());
Assert.IsTrue(response.Headers.TryGetValue("ETag", out string etag));
Assert.AreEqual(version, etag);
Assert.IsTrue(hasChanged);
}

[Test]
public async Task HasChangedValuesMatch()
{
var response = new MockResponse(200);

string version = Guid.NewGuid().ToString();
var versionedSetting = s_testSetting.Clone();
versionedSetting.ETag = new ETag(version);

response.SetContent(SerializationHelpers.Serialize(versionedSetting, SerializeSetting));
response.AddHeader(new HttpHeader("ETag", versionedSetting.ETag.ToString()));

var mockTransport = new MockTransport(response);
ConfigurationClient service = CreateTestService(mockTransport);

bool hasChanged = await service.HasChangedAsync(versionedSetting);
MockRequest request = mockTransport.SingleRequest;

AssertRequestCommon(request);
Assert.AreEqual(RequestMethod.Head, request.Method);
Assert.AreEqual($"https://contoso.appconfig.io/kv/test_key?label=test_label&api-version={s_version}", request.Uri.ToString());
Assert.IsTrue(response.Headers.TryGetValue("ETag", out string etag));
Assert.AreEqual(version, etag);
Assert.IsFalse(hasChanged);
}

[Test]
public async Task HasChangedValuesDontMatch()
{
var response = new MockResponse(200);

string version1 = Guid.NewGuid().ToString();
var version1Setting = s_testSetting.Clone();
version1Setting.ETag = new ETag(version1);

string version2 = Guid.NewGuid().ToString();
var version2Setting = s_testSetting.Clone();
version2Setting.ETag = new ETag(version2);

response.SetContent(SerializationHelpers.Serialize(version2Setting, SerializeSetting));
response.AddHeader(new HttpHeader("ETag", version2Setting.ETag.ToString()));

var mockTransport = new MockTransport(response);
ConfigurationClient service = CreateTestService(mockTransport);

bool hasChanged = await service.HasChangedAsync(version1Setting);
MockRequest request = mockTransport.SingleRequest;

AssertRequestCommon(request);
Assert.AreEqual(RequestMethod.Head, request.Method);
Assert.AreEqual($"https://contoso.appconfig.io/kv/test_key?label=test_label&api-version={s_version}", request.Uri.ToString());
Assert.IsTrue(response.Headers.TryGetValue("ETag", out string etag));
Assert.AreEqual(version2, etag);
Assert.IsTrue(hasChanged);
}

[Test]
public async Task HasChangedValueNotFound()
{
var response = new MockResponse(404);

string version = Guid.NewGuid().ToString();
var versionedSetting = s_testSetting.Clone();
versionedSetting.ETag = new ETag(version);

var mockTransport = new MockTransport(response);
ConfigurationClient service = CreateTestService(mockTransport);

bool hasChanged = await service.HasChangedAsync(versionedSetting);

MockRequest request = mockTransport.SingleRequest;

AssertRequestCommon(request);
Assert.AreEqual(RequestMethod.Head, request.Method);
Assert.AreEqual($"https://contoso.appconfig.io/kv/test_key?label=test_label&api-version={s_version}", request.Uri.ToString());
Assert.IsTrue(hasChanged);
}

[Test]
public async Task SetReadOnly()
{
Expand Down