Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

253: C# Support for HTTP Scanning #2

Merged
merged 7 commits into from

2 participants

@paulj

Initial support for HTTP scanning via C#.

...t/Net.LShift.Diffa.Participants/AggregationBuilder.cs
((47 lines not shown))
+ /// </summary>
+ /// <param name="attrName">the name of the attribute</param>
+ public void MaybeAddDateAggregation(string attrName) {
+ var attrGranularity = req[attrName + "-granularity"];
+ if (attrGranularity != null) {
+ switch (attrGranularity) {
+ case "daily":
+ result.Add(new DailyCategoryFunction(attrName));
+ break;
+ case "monthly":
+ result.Add(new MonthlyCategoryFunction(attrName));
+ break;
+ case "yearly":
+ result.Add(new YearlyCategoryFunction(attrName));
+ break;
+ }

Perhaps the default branch of this switch statement should throw some kind of strongly typed exception that propagates a 404 on the WCF layer.

@paulj
paulj added a note

I've added handling for this to generate 400 errors with sensible messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...t/Net.LShift.Diffa.Participants/AggregationBuilder.cs
((56 lines not shown))
+ case "monthly":
+ result.Add(new MonthlyCategoryFunction(attrName));
+ break;
+ case "yearly":
+ result.Add(new YearlyCategoryFunction(attrName));
+ break;
+ }
+ }
+ }
+
+ /// <summary>
+ /// Attempt to add a by name aggregation for the given attribute.
+ /// </summary>
+ /// <param name="attrName">the name of the attribute</param>
+ public void MaybeAddByNameAggregation(string attrName) {
+ var attrGranularity = req[attrName + "-granularity"];

Given that this function (and the preceding function) are building something from strings, is it worth factoring this out a bit more to avoid duplication? This would tie in with the previous comment to throw an exception for an unmatched string.

@paulj
paulj added a note

Granularity field extraction is now a common function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ift.Diffa.Participants.Test/ConstraintsBuilderTest.cs
((99 lines not shown))
+
+ [Test]
+ public void ShouldAddSetConstraintWhenMultipleValuesArePresent() {
+ var req = new NameValueCollection {{"someString", "a"}, {"someString", "b"}, {"someString", "c"}};
+ var builder = new ConstraintsBuilder(req);
+
+ builder.MaybeAddSetConstraint("someString");
+ Assert.AreEqual(1, builder.ToList().Count);
+ Assert.IsInstanceOf(typeof (SetQueryConstraint), builder.ToList()[0]);
+
+ var c = (SetQueryConstraint) builder.ToList()[0];
+ var expected = new HashSet<String> { "a", "b", "c" };
+ Assert.That(expected, Is.EqualTo(c.Values).AsCollection);
+ }
+ }
+}

Do you think it is worth adding a test case with two different types of constraints, e.g. a date and set?

@paulj
paulj added a note

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@0x6e6562 0x6e6562 commented on the diff
...ant/Net.LShift.Diffa.Participants/IQueryConstraint.cs
@@ -46,6 +47,45 @@ namespace Net.LShift.Diffa.Participants
}
}
+

Do prefix constraints feature in this library?

@paulj
paulj added a note

They didn't, but they do now. Note that I'm only supporting them in the new portion of the library (ie, they don't feature in any of the registries) since I intend to deprecate those quite soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
....LShift.Diffa.Participants/BaseScanningParticipant.cs
((30 lines not shown))
+ // Retrieve the query parameters for the request
+ var queryParameters = WebOperationContext.Current.IncomingRequest.UriTemplateMatch.QueryParameters;
+
+ // Determine constraints and aggregation
+ var constraints = DetermineConstraints(queryParameters);
+ var aggregations = DetermineAggregations(queryParameters);
+
+ try {
+ var results = Query(constraints, aggregations);
+
+ // Ensure that we have a content type set on the response
+ WebOperationContext.Current.OutgoingResponse.ContentType = "application/json";
+
+ return new MemoryStream(Encoding.UTF8.GetBytes(new JArray(results.Select(r => r.ToJObject())).ToString()));
+ } catch (Exception ex) {
+ Console.WriteLine(ex);

Would it not be more useful to use nlog instead of the console?

@paulj
paulj added a note

NLog currently isn't a dependency of the library, and I'm not quite sure if we want to make it one. I've removed the WriteLine in favour of more comprehensive error handling, and letting the exception be thrown out will allow the standard WCF error logs to see the message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@0x6e6562 0x6e6562 merged commit e5c8caf into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.