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.

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

Create Access Key #129

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
35 changes: 35 additions & 0 deletions Keen.NET.Test/AccessKeyMock.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using Keen.Core.AccessKey;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json.Linq;
using Keen.Core;

namespace Keen.Net.Test
{
/// <summary>
/// AccessKeyMock provides an implementation of IAccessKeys with a constructor that
/// accepts delegates for each of the interface methods.
/// The purpose of this is to allow test methods to set up a customized
/// IAccessKeys for each test.
/// </summary>
class AccessKeysMock : IAccessKeys
{
private readonly IProjectSettings _settings;
private readonly Func<AccessKey, IProjectSettings, JObject> _createAccessKey;

public AccessKeysMock(IProjectSettings projSettings,
Func<AccessKey, IProjectSettings, JObject> createAccessKey = null)
{
_settings = projSettings;
_createAccessKey = createAccessKey ?? ((p, k) => new JObject());
}

public Task<JObject> CreateAccessKey(AccessKey accesskey)
{
return Task.Run(() => _createAccessKey(accesskey, _settings));
}
}
}
1 change: 1 addition & 0 deletions Keen.NET.Test/Keen.NET.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
<Compile Include="..\SharedVersionInfo.cs">
<Link>Properties\SharedVersionInfo.cs</Link>
</Compile>
<Compile Include="AccessKeyMock.cs" />
<Compile Include="AddOnsTest.cs" />
<Compile Include="EventCacheTest.cs" />
<Compile Include="EventCollectionMock.cs" />
Expand Down
38 changes: 38 additions & 0 deletions Keen.NET.Test/KeenClientTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Keen.Core;
using Keen.Core.AccessKey;
using Keen.Core.EventCache;
using Moq;
using Newtonsoft.Json.Linq;
Expand Down Expand Up @@ -45,6 +46,43 @@ public static void ResetEnv()
}
}

[TestFixture]
public class AccessKeysTest : TestBase
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should break this out into another file.

{
[Test]
public void CreateAccessKey_Success()
{
var settings = new ProjectSettingsProvider(projectId: "X", masterKey: SettingsEnv.MasterKey); // Replace X with respective value
var client = new KeenClient(settings);

if (UseMocks)
client.AccessKeys = new AccessKeysMock(settings,
Copy link
Contributor

Choose a reason for hiding this comment

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

If the idea is just to do unit testing on the CreateAccessKey() method of the KeenClient, we can just leverage Moq instead of creating a separate class, which might be simpler.

createAccessKey: new Func<AccessKey, IProjectSettings, JObject>((e,p) =>
{
Assert.True(p == settings, "Incorrect Settings");
Assert.NotNull(e.Name, "Expected a name for the newly created Key");
Assert.NotNull(e.Permitted, "Expected a list of high level actions this key can perform");
Assert.NotNull(e.Options, "Expected an object containing more details about the key’s permitted and restricted functionality");
if ((p == settings) && (e.Name == "TestAccessKey") && (e.IsActive) && e.Permitted.First() == "queries" && e.Options.CachedQueries.Allowed.First() == "my_cached_query")
return new JObject();
else
throw new Exception("Unexpected value");
}));

HashSet<string> permissions = new HashSet<string>() { "queries" };
List<Core.Query.QueryFilter> qFilters = new List<Core.Query.QueryFilter>() { new Core.Query.QueryFilter("customer.id", Core.Query.QueryFilter.FilterOperator.Equals(), "asdf12345z") };
Copy link
Contributor

Choose a reason for hiding this comment

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

If/when you create a new file for Access Key tests, I'd recommend adding using Keen.Core.Query; and dispensing with the explicit namespace here.

CachedQueries cachedQuaries = new CachedQueries();
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "cachedQuaries"->"cachedQueries"

cachedQuaries.Allowed = new HashSet<string>() { "my_cached_query" };
Options options = new Options()
{
Queries = new Quaries { Filters = qFilters },
CachedQueries = cachedQuaries
};
Assert.DoesNotThrow(() => client.CreateAccessKey(new AccessKey("TestAccessKey", true, permissions, options)));
}

}

[TestFixture]
public class BulkEventTest : TestBase
{
Expand Down
79 changes: 79 additions & 0 deletions Keen/AccessKey/AccessKey.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
using Keen.Core.Query;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Newtonsoft.Json.Serialization;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Keen.Core.AccessKey
{
/// <summary>
/// Modal for AccessKey object
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "Modal"->"Model", and we should elaborate on XML doc comments on any public members.

/// </summary>
public class AccessKey
{
public string Name { get; set; }
public bool IsActive { get; set; }
public ISet<string> Permitted { get; set; }
public string Key { get; set; }
public Options Options { get; set; }

internal AccessKey(string name, bool isActive, ISet<string> permitted, Options options)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the properties all remain public, is it useful to have an internal constructor instead of using object initializer notation and giving the Key auto-prop a default?

{
this.Key = null; //Not needed for the creation of key
this.Name = name;
this.IsActive = isActive;
this.Permitted = permitted;
this.Options = options;
}


}

public class SavedQueries
Copy link
Contributor

Choose a reason for hiding this comment

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

All these public types should have XML doc comments.

{
public ISet<string> Blocked { get; set; }
public IEnumerable<QueryFilter> Filters { get; set; }
public ISet<string> Allowed { get; set; }
}

public class Quaries
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "Quaries"->"Queries"

{
public IEnumerable<QueryFilter> Filters { get; set; }
}

public class Writes
{
public dynamic Autofill { get; set; } // "customer": { "id": "93iskds39kd93id", "name": "Ada Corp." }
}

public class Datasets
{
public IEnumerable<string> Operations { get; set; }
public IDictionary<string, AllowedDatasetIndexes> Allowed { get; set; }
public ISet<string> Blocked { get; set; }
}

public class AllowedDatasetIndexes
{
public Tuple<string, string> IndexBy { get; set;}
}

public class CachedQueries
{
public ISet<string> Blocked { get; set; }
public ISet<string> Allowed { get; set; }
}

public class Options
{
public SavedQueries SavedQueries { get; set; }
public Writes Writes { get; set; }
public Datasets Datasets { get; set; }
public CachedQueries CachedQueries { get; set; }
public Quaries Queries { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "Quaries"->"Queries"

}
}
104 changes: 104 additions & 0 deletions Keen/AccessKey/AccessKeys.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Newtonsoft.Json.Serialization;
using System;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to organize and remove unnecessary usings.

using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Keen.Core.AccessKey
{
/// <summary>
/// AccessKeys implements the IAccessKeys interface which represents the Keen.IO Access Key API methods.
/// </summary>
public class AccessKeys : IAccessKeys
{
private readonly IKeenHttpClient _keenHttpClient;
private readonly string _acceskeyRelativeUrl;
Copy link
Contributor

@masojus masojus Nov 3, 2017

Choose a reason for hiding this comment

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

typo: "_acceskey..."->"_accessKey...",

private readonly string _readKey;
private readonly string _masterKey;

internal AccessKeys(IProjectSettings prjSettings,
IKeenHttpClientProvider keenHttpClientProvider)
{
if (null == prjSettings)
{
throw new ArgumentNullException(nameof(prjSettings),
"Project Settings must be provided.");
}

if (null == keenHttpClientProvider)
{
throw new ArgumentNullException(nameof(keenHttpClientProvider),
"A KeenHttpClient provider must be provided.");
}

if (string.IsNullOrWhiteSpace(prjSettings.KeenUrl) ||
!Uri.IsWellFormedUriString(prjSettings.KeenUrl, UriKind.Absolute))
{
throw new KeenException(
"A properly formatted KeenUrl must be provided via Project Settings.");
}

var serverBaseUrl = new Uri(prjSettings.KeenUrl);
_keenHttpClient = keenHttpClientProvider.GetForUrl(serverBaseUrl);
_acceskeyRelativeUrl = KeenHttpClient.GetRelativeUrl(prjSettings.ProjectId,
KeenConstants.AccessKeyResource);

_readKey = prjSettings.ReadKey;
_masterKey = prjSettings.MasterKey;
}

public async Task<JObject> CreateAccessKey(AccessKey accesskey)
{
if (string.IsNullOrWhiteSpace(_masterKey))
{
throw new KeenException("An API WriteKey is required to add events.");
}

var xcontent = accesskey.ToString();
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this xcontent var we have here?


DefaultContractResolver contractResolver = new DefaultContractResolver
{
NamingStrategy = new SnakeCaseNamingStrategy()
};

var content = JsonConvert.SerializeObject(accesskey, new JsonSerializerSettings
{
ContractResolver = contractResolver,
Formatting = Formatting.Indented
}).ToSafeString();

var responseMsg = await _keenHttpClient
.PostAsync(_acceskeyRelativeUrl, _masterKey, content)
.ConfigureAwait(continueOnCapturedContext: false);

var responseString = await responseMsg
.Content
.ReadAsStringAsync()
.ConfigureAwait(continueOnCapturedContext: false);

JObject jsonResponse = null;

try
{
jsonResponse = JObject.Parse(responseString);
}
catch (Exception)
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this because you were seeing a 404 error or something similar and it's actually OK in this case to swallow the exception? Then will we throw below? I know we do this elsewhere, and we should really keep the inner exception around for context, but that's another issue. At least if that's the case, let's put a comment here explaining the empty catch(){} block.

{
}
if (!responseMsg.IsSuccessStatusCode)
{
throw new KeenException("AddEvents failed with status: " + responseMsg.StatusCode);
}

if (null == jsonResponse)
{
throw new KeenException("AddEvents failed with empty response from server.");
}

return jsonResponse;
}
}
}
19 changes: 19 additions & 0 deletions Keen/AccessKey/IAccessKeys.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Keen.Core.AccessKey
{
public interface IAccessKeys
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll want to flesh out XML doc comments here.

{
/// <summary>
/// Creates an Access Key
/// </summary>
/// <param name="accesskey"></param>
/// <returns></returns>
Task<JObject> CreateAccessKey(AccessKey accesskey);
Copy link
Contributor

@masojus masojus Nov 3, 2017

Choose a reason for hiding this comment

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

We prefer Task-returning methods to use the ...Async(...) naming guideline.

}
}
3 changes: 3 additions & 0 deletions Keen/Keen.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@
<Compile Include="..\SharedVersionInfo.cs">
<Link>Properties\SharedVersionInfo.cs</Link>
</Compile>
<Compile Include="AccessKey\AccessKey.cs" />
<Compile Include="AccessKey\AccessKeys.cs" />
<Compile Include="AccessKey\IAccessKeys.cs" />
<Compile Include="DataEnrichment\EventAddOn.cs" />
<Compile Include="HttpClientCache.cs" />
<Compile Include="IHttpClientProvider.cs" />
Expand Down
36 changes: 36 additions & 0 deletions Keen/KeenClient.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Keen.Core.DataEnrichment;
using Keen.Core.EventCache;
using Keen.Core.Query;
using Keen.Core.AccessKey;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -48,6 +49,12 @@ public class KeenClient
/// </summary>
public IQueries Queries { get; set; }

/// <summary>
/// AccessKeys provides direct access to the Keen.IO Access Keys API methods.
/// The default implementation can be overridden by setting a new implementation here.
/// </summary>
public IAccessKeys AccessKeys { get; set; }

/// <summary>
/// Add a static global property. This property will be added to
/// every event.
Expand Down Expand Up @@ -116,6 +123,7 @@ private KeenClient(IProjectSettings prjSettings,
EventCollection = new EventCollection(_prjSettings, keenHttpClientProvider);
Event = new Event(_prjSettings, keenHttpClientProvider);
Queries = new Queries(_prjSettings, keenHttpClientProvider);
AccessKeys = new AccessKeys(_prjSettings, keenHttpClientProvider);
}

/// <summary>
Expand Down Expand Up @@ -970,5 +978,33 @@ public IEnumerable<QueryIntervalValue<IEnumerable<QueryGroupValue<IDictionary<st
throw ex.TryUnwrap();
}
}

/// <summary>
/// </summary>
public void CreateAccessKey(AccessKey.AccessKey accessKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need the namespace qualification?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes since namespace and class have the same name, even though I used the namespace, C# fails to understand, AccessKey is a class.

Copy link
Contributor

Choose a reason for hiding this comment

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

If that's the case it seems like we should rename to AccessKeys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the namespace? We have another class called AccessKeys

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, or does that cause other problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah in that sense, we may need to go for AccessKeys = new AccessKeys.AccessKeys(_prjSettings, keenHttpClientProvider); in the KeenClient.cs

{
try
{
CreateAccessKeyAsync(accessKey).Wait();
}
catch (AggregateException ex)
{
Debug.WriteLine(ex.TryUnwrap());
throw ex.TryUnwrap();
}
}

///<summary>
///</summary>
private async Task<JObject> CreateAccessKeyAsync(AccessKey.AccessKey accessKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need the namespace qualification?

{
if (null == accessKey)
throw new KeenException("Access Key required");

var createdKey = await AccessKeys.CreateAccessKey(accessKey)
.ConfigureAwait(false);

return createdKey;
}
}
}
3 changes: 3 additions & 0 deletions Keen/KeenConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ public class KeenConstants
private const string eventsResource = "events";
public static string EventsResource { get { return eventsResource; } protected set { ;} }

private const string accesskeyResource = "keys";
public static string AccessKeyResource { get { return accesskeyResource; } protected set {; } }

private const string queriesResource = "queries";
public static string QueriesResource { get { return queriesResource; } protected set { ;} }

Expand Down