Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Initial checkin for WinMD WindowsAzure API #3

Closed
wants to merge 12 commits into from

2 participants

@aliakb

Here's the initial checkin for the WinMD WindowsAzure service layer API. The changes include the necessary infrastructure for making calls to Windows Azure, plus functionality for managing service bus queues.

Highlights:

  • Assembly name is Microsoft.WindowsAzure.ServiceLayer.dll
  • All common types reside in Microsoft.WindowsAzure.ServiceLayer namespace
  • All service bus types reside in Microsoft.WindowsAzure.ServiceLayer.ServiceBus namespace
  • Microsoft.WindowsAzure.ServiceLayer.ServiceBus.ServiceBusService class represents an entry point to SB functionality and serves as a factory for establishing connections to SB
  • Types for manipulating SB primitives fall into the following two categories:
    • Settings (QueueSetting). These are input parameters for creating SB objects. Such classes contain a lot of nullable value attributes for specifying parameters of SB objects. A default value will be used if a parameter is not set.
    • Infos (QueueInfo). These are output parameters returning information of SB objects. A call to get all queues from the service bus returns a collection of these. A call to create a queue returns QueueInfo describing that queue.

Please note that the API is incomplete and currently includes only very basic methods for operating with queues.

@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...ft.WindowsAzure.ServiceLayer/AzureServiceException.cs
((9 lines not shown))
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ class AzureServiceException: Exception
@ghost
ghost added a note

Would nice to be specific with public/internal/private. Also mark it abstract if it isn't designed to be directly instantiated.

@aliakb
aliakb added a note

Is there a class that cannot be instantiated that is not marked as abstract? I usually do that, but I may have missed something.

@tg-msft
tg-msft added a note

We should be consistent with Azure vs. WindowsAzure in our naming.

@aliakb
aliakb added a note

Renamed to WindowsAzureServiceException. However, I am not sure if I want/need to keep this class - need to figure out whole error handling story in WinMD files. It turned out that you cannot declare public exception classes in WinMD libraries.

@tg-msft
tg-msft added a note

You should update your pull request (just checkin again to the same branch you submitted the pull from and github will propagate so we see updates in the PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...ft.WindowsAzure.ServiceLayer/AzureServiceException.cs
@@ -0,0 +1,27 @@
+/*
@ghost
ghost added a note

Would like to consult with Ted here on the headers, for C# I almost always prefer proper two-slash comments including for license headers.

But if there's team precedence here, I'm OK with this.

@aliakb
aliakb added a note

These were copied from a java file. I'll check with Ted tomorrow if he has C#-specific headers.

@aliakb
aliakb added a note

All files have been updated with the proper heading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...crosoft.WindowsAzure.ServiceLayer/HttpErrorHandler.cs
((8 lines not shown))
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.Net.Http;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ class HttpErrorHandler: MessageProcessingHandler
@ghost
ghost added a note

Same feedback here.

@aliakb
aliakb added a note

All classes and class members have been updated with explicitly specified visibility attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...crosoft.WindowsAzure.ServiceLayer/HttpErrorHandler.cs
((21 lines not shown))
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ class HttpErrorHandler: MessageProcessingHandler
+ {
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ internal HttpErrorHandler()
+ : base()
+ {
+ }
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="innerHandler">Inner HTTP handler</param>
@ghost
ghost added a note

It is another small nit, but consistency in XML comments is good... so please end with a period here: 'Inner HTTP handler.'

@aliakb
aliakb added a note

Updated punctuation on all param descriptions and return values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...crosoft.WindowsAzure.ServiceLayer/HttpErrorHandler.cs
((28 lines not shown))
+ internal HttpErrorHandler()
+ : base()
+ {
+ }
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="innerHandler">Inner HTTP handler</param>
+ internal HttpErrorHandler(HttpMessageHandler innerHandler)
+ : base(innerHandler)
+ {
+ }
+
+ /// <summary>
+ /// Processes outhoing HTTP requests.
@ghost
ghost added a note

Spelling error.

@aliakb
aliakb added a note

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...crosoft.WindowsAzure.ServiceLayer/HttpErrorHandler.cs
((33 lines not shown))
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="innerHandler">Inner HTTP handler</param>
+ internal HttpErrorHandler(HttpMessageHandler innerHandler)
+ : base(innerHandler)
+ {
+ }
+
+ /// <summary>
+ /// Processes outhoing HTTP requests.
+ /// </summary>
+ /// <param name="request">Request</param>
+ /// <param name="cancellationToken">Cancellation token</param>
+ /// <returns>Processed HTTP request</returns>
+ protected override HttpRequestMessage ProcessRequest(HttpRequestMessage request, System.Threading.CancellationToken cancellationToken)
@ghost
ghost added a note

Consider using the System.Threading namespace and removing the explicit declaration.

@aliakb
aliakb added a note

Fixed everywhere in the project. That namespace is not added by default, and when VS generates method, it uses full class name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...crosoft.WindowsAzure.ServiceLayer/HttpErrorHandler.cs
((31 lines not shown))
+ }
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="innerHandler">Inner HTTP handler</param>
+ internal HttpErrorHandler(HttpMessageHandler innerHandler)
+ : base(innerHandler)
+ {
+ }
+
+ /// <summary>
+ /// Processes outhoing HTTP requests.
+ /// </summary>
+ /// <param name="request">Request</param>
+ /// <param name="cancellationToken">Cancellation token</param>
@ghost
ghost added a note

This one is fun, there are 2 correct ways to spell Cancellation/Cancelation.

Should check what the .NET framework naming guidelines say. I honestly don't remember. Windows phone got this wrong and so they have both in the API..

@aliakb
aliakb added a note

That name was autogenerated by VS when I told it to implement abstract parent class.

@ghost
ghost added a note

Great, thanks. That's the spelling we will stick with!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...crosoft.WindowsAzure.ServiceLayer/HttpErrorHandler.cs
((48 lines not shown))
+ protected override HttpRequestMessage ProcessRequest(HttpRequestMessage request, System.Threading.CancellationToken cancellationToken)
+ {
+ // We're not interested in outgoing requests; do nothing.
+ return request;
+ }
+
+ /// <summary>
+ /// Processes incoming HTTP responses.
+ /// </summary>
+ /// <param name="response">HTTP response</param>
+ /// <param name="cancellationToken">Cancellation token</param>
+ /// <returns>Processed HTTP response</returns>
+ protected override HttpResponseMessage ProcessResponse(HttpResponseMessage response, System.Threading.CancellationToken cancellationToken)
+ {
+ if (!response.IsSuccessStatusCode)
+ throw new AzureServiceException();
@ghost
ghost added a note

My preference is to always have even simple logic blocks contained with curly braces.

@tg-msft
tg-msft added a note

I'm not sure if this is the right place or way to throw this exception:
1. What does the stack trace look like when something blows up in the pipeline? If it's really confusing to a user where the error came from, we might think about at least rethrowing from within the service itself to rebase the call stack.
2. Some methods return specific codes in the 400s to indicate what type of failure. Furthermore, most failing status codes will come with an XML error payload that describes exactly what went wrong (which is probably what we should use as the message of the exception).

@aliakb
aliakb added a note

Error handling in asynchronous methods is an interesting story. You'll get an AggregateException when calling Task.Wait or when obtaining Task.Result property. That exception provides you access to all exceptions that happened while executing the call.

Note that you cannot define your own public exceptions.

@tg-msft
tg-msft added a note

You can definitely define your own exceptions - it just might not be possible to pass them across WinRT boundaries. In that case, if you want to detect them later on, you should define your own and then a WrapExceptionForWinRT static method that transforms the exception into something you can rethrow with a useful callstack.

@aliakb
aliakb added a note

OK, thanks for the info. Unless you object, I am going to submit the code as is - and take care of the static method in one of the following checkins. The main idea, however, still remains the same - HTTP errors get translated into exceptions by a handler; when exception occurs, further tasks in the chain are not executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...ayer/Microsoft.WindowsAzure.ServiceLayer/HttpQuery.cs
((14 lines not shown))
+ */
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ /// <summary>
+ /// Helper class for processing HTTP query strings.
+ /// </summary>
+ class HttpQuery
+ {
+ Dictionary<string, string> _values = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
@ghost
ghost added a note

Explicit private indicator would be nice for the field.

@aliakb
aliakb added a note

Fixed for all class members.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...ayer/Microsoft.WindowsAzure.ServiceLayer/HttpQuery.cs
((27 lines not shown))
+ class HttpQuery
+ {
+ Dictionary<string, string> _values = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="queryString">Query string</param>
+ internal HttpQuery(string queryString)
+ {
+ string[] pairs = queryString.Split('&');
+
+ foreach (string pair in pairs)
+ {
+ int pos = pair.IndexOf('=');
+ string name = pair.Substring(0, pos);
@ghost
ghost added a note

For robustness, might want to check pos >= 0 - or at least validate what will happen if an invalid query string is passed in. Might be worth using a more robust query string parsing library - we have some in ASP.NET and other parts of C# / libraries that we could consider using.

@aliakb
aliakb added a note

I was looking for classes that could parse HTTP queries and found one only in ASP.Net. I did not want, however, to add a dependency on that library only to use their parser.

Missing characters in the query string would mean malformed input. Processing that would require us to abort execution anyway. If we don't check return values, we'll get something like an "index out of range" exception. If we do - we can throw something nicer. However, I am not sure it's worth the effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...ayer/Microsoft.WindowsAzure.ServiceLayer/HttpQuery.cs
((38 lines not shown))
+
+ foreach (string pair in pairs)
+ {
+ int pos = pair.IndexOf('=');
+ string name = pair.Substring(0, pos);
+ string value = pair.Substring(pos + 1);
+ _values.Add(name, value);
+ }
+ }
+
+ /// <summary>
+ /// Gets parameter by name.
+ /// </summary>
+ /// <param name="parameterName">Parameter name</param>
+ /// <returns>Parameter value</returns>
+ internal string this[string parameterName] { get { return _values[parameterName]; } }
@ghost
ghost added a note

My preference would be to break this out into many lines to improve readability.

@aliakb
aliakb added a note

I usually do not split simple properties that just return values, into multiple lines. Keeping such properties on a single line results in denser code, which is not a bad thing, too.

@tg-msft
tg-msft added a note

I'm a fan of multiple lines here too - mostly because that's the default StyleCop/SourceAnalysis setting and then everything looks delightfully uniform.

@aliakb
aliakb added a note

Code analysis in VS IDE with default settings does not complain about single-line properties. It did complain about double closing pattern for streams/readers, which I fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
....WindowsAzure.ServiceLayer/Properties/AssemblyInfo.cs
@@ -0,0 +1,29 @@
+using System.Reflection;
@ghost
ghost added a note

Need the copyright header in here as well.

@aliakb
aliakb added a note

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...soft.WindowsAzure.ServiceLayer/SerializationHelper.cs
((75 lines not shown))
+ using (XmlReader xmlReader = XmlReader.Create(stringReader))
+ {
+ T deserializedObject = (T)serializer.ReadObject(xmlReader);
+ itemAction(item, deserializedObject);
+ return deserializedObject;
+ }
+ }
+
+ /// <summary>
+ /// Serializes given object.
+ /// </summary>
+ /// <param name="item">Object to serialize</param>
+ /// <returns>Serialized representation</returns>
+ static internal string Serialize(object item)
+ {
+ // Serialize the content
@ghost
ghost added a note

comment feels redundant

@aliakb
aliakb added a note

Added more comments for the following step. Now this one doesn't feel lonely anymore :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...soft.WindowsAzure.ServiceLayer/SerializationHelper.cs
((92 lines not shown))
+
+ using (MemoryStream stream = new MemoryStream())
+ {
+ DataContractSerializer serializer = new DataContractSerializer(item.GetType());
+ serializer.WriteObject(stream, item);
+
+ stream.Flush();
+ stream.Seek(0, SeekOrigin.Begin);
+ using (StreamReader reader = new StreamReader(stream))
+ {
+ itemXml = reader.ReadToEnd();
+ }
+ }
+
+ SyndicationContent content = new SyndicationContent();
+ content.Type = "application/xml";
@ghost
ghost added a note

I would prefer this be a string constant.

@aliakb
aliakb added a note

I have moved most of strings into a dedicated Constants class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...sAzure.ServiceLayer/ServiceBus/ServiceBusRestProxy.cs
((54 lines not shown))
+ ServiceConfig = serviceOptions;
+ HttpMessageHandler chain = new HttpErrorHandler(
+ new WrapAuthenticationHandler(serviceOptions));
+ Channel = new HttpClient(chain);
+ }
+
+ /// <summary>
+ /// Gets all available queues in the namespace.
+ /// </summary>
+ /// <returns>All queues in the namespace</returns>
+ IAsyncOperation<IEnumerable<QueueInfo>> IServiceBusService.ListQueuesAsync()
+ {
+ Uri uri = new Uri(ServiceConfig.ServiceBusUri, "$Resources/Queues");
+ HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, uri);
+
+ return Channel.SendAsync(request)
@ghost
ghost added a note

Some LINQ style guidelines would recommend breaking the .SendAsync into a new line to be consistent with the rest of this LINQ chain.

@aliakb
aliakb added a note

That implies different coding styles for "instance.Method(...)" and "instance.Method(...).ContinueWith(....)" calls - depending on the presence of ".ContinueWith" we would have to put the first call either on a single line or on two lines.

@tg-msft
tg-msft added a note

Yes, we have different coding styles for single method calls vs. chained sequences of method calls.

@aliakb
aliakb added a note

OK; fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...sAzure.ServiceLayer/ServiceBus/ServiceBusRestProxy.cs
((66 lines not shown))
+ Uri uri = new Uri(ServiceConfig.ServiceBusUri, "$Resources/Queues");
+ HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, uri);
+
+ return Channel.SendAsync(request)
+ .ContinueWith<IEnumerable<QueueInfo>>(r => { return GetQueues(r.Result); }, TaskContinuationOptions.OnlyOnRanToCompletion)
+ .AsAsyncOperation<IEnumerable<QueueInfo>>();
+ }
+
+ /// <summary>
+ /// Gets the queue with the given name.
+ /// </summary>
+ /// <param name="queueName">Name of the queue</param>
+ /// <returns>Queue data</returns>
+ IAsyncOperation<QueueInfo> IServiceBusService.GetQueueAsync(string queueName)
+ {
+ if (queueName == null)
@ghost
ghost added a note

use curly braces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...sAzure.ServiceLayer/ServiceBus/ServiceBusRestProxy.cs
((178 lines not shown))
+ return Task.Factory.StartNew(() => SetBody(request, queueSettings))
+ .ContinueWith<HttpResponseMessage>(tr => { return Channel.SendAsync(request).Result; }, TaskContinuationOptions.OnlyOnRanToCompletion)
+ .ContinueWith<QueueInfo>(tr => { return GetQueue(tr.Result); }, TaskContinuationOptions.OnlyOnRanToCompletion)
+ .AsAsyncOperation<QueueInfo>();
+
+ }
+
+ /// <summary>
+ /// Serializes given object and sets the request's body.
+ /// </summary>
+ /// <param name="request">Target request</param>
+ /// <param name="bodyObject">Object to serialize</param>
+ void SetBody(HttpRequestMessage request, object bodyObject)
+ {
+ string content = SerializationHelper.Serialize(bodyObject);
+ request.Content = new StringContent(content, Encoding.UTF8, "application/atom+xml");
@ghost
ghost added a note

another constant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...Azure.ServiceLayer/ServiceBus/ServiceConfiguration.cs
((57 lines not shown))
+ /// </summary>
+ internal Uri ScopeHostUri { get; private set; }
+
+ /// <summary>
+ /// Constructor with explicitly specified options.
+ /// </summary>
+ /// <param name="serviceNamespace">Service namespace</param>
+ /// <param name="userName">User name for authentication</param>
+ /// <param name="password">Password for authentication</param>
+ internal ServiceConfiguration(string serviceNamespace, string userName, string password)
+ {
+ ServiceNamespace = serviceNamespace;
+ UserName = userName;
+ Password = password;
+
+ string stringUri = string.Format(CultureInfo.InvariantCulture, "https://{0}.servicebus.windows.net/", ServiceNamespace);
@ghost
ghost added a note

pull out into consistent format constants that are available at the top of the file and/or a centralized constants type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
...oft.WindowsAzure.ServiceLayer/ServiceBus/WrapToken.cs
((48 lines not shown))
+ internal bool IsExpired { get { return DateTime.Now > _expirationDate; } }
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="resourcePath">Path of the authenticated resource</param>
+ /// <param name="response">HTTP response with the token</param>
+ internal WrapToken(string resourcePath, HttpResponseMessage response)
+ {
+ Debug.Assert(response.IsSuccessStatusCode);
+ Scope = resourcePath;
+ string content = response.Content.ReadAsStringAsync().Result;
+
+ HttpQuery query = new HttpQuery(content);
+ Token = string.Format(CultureInfo.InvariantCulture, "WRAP access_token=\"{0}\"", WebUtility.UrlDecode(query["wrap_access_token"]));
+ _expirationDate = DateTime.Now + TimeSpan.FromSeconds(int.Parse(query["wrap_access_token_expires_in"]) / 2);
@ghost
ghost added a note

Having a raw int.Parse inside here could lead to issues. Would be best to use TryParse or some other flow.

@aliakb
aliakb added a note

It will throw in case of invalid input format. Do we want to be more specific and tell the users that the we got an unexpected response from the authentication service?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Aliaksei Bat... added some commits
...ayer/Microsoft.WindowsAzure.ServiceLayer/HttpQuery.cs
((22 lines not shown))
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ /// <summary>
+ /// Helper class for processing HTTP query strings.
+ /// </summary>
+ class HttpQuery
+ {
+ Dictionary<string, string> _values = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="queryString">Query string</param>
+ internal HttpQuery(string queryString)
+ {
+ string[] pairs = queryString.Split('&');
@tg-msft
tg-msft added a note

Has this already been URL decoded? If not, your logic below could get a little messy. There's an equivalent of http://msdn.microsoft.com/en-us/library/adwtk1fy.aspx tucked away somewhere in the core libraries in the new platform. You might also need to trim a leading ? depending on where you get the querystring from.

@aliakb
aliakb added a note

It has been. The class is used in a single place - to extract WRAP token details from the response of authentication purpose.

The class for encoding/decoding is called System.Net.WebUtility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ayer/Microsoft.WindowsAzure.ServiceLayer/HttpQuery.cs
((12 lines not shown))
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ /// <summary>
+ /// Helper class for processing HTTP query strings.
+ /// </summary>
+ class HttpQuery
@tg-msft
tg-msft added a note

The name makes me think this class does something different. Perhaps HttpQueryStringParameters would be more descriptive.

@aliakb
aliakb added a note

Renamed into HttpQueryStringParser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
....WindowsAzure.ServiceLayer/Properties/AssemblyInfo.cs
@@ -0,0 +1,29 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+// General Information about an assembly is controlled through the following
+// set of attributes. Change these attribute values to modify the information
+// associated with an assembly.
+[assembly: AssemblyTitle("Microsoft.WindowsAzure.ServiceLayer")]
+[assembly: AssemblyDescription("")]
+[assembly: AssemblyConfiguration("")]
+[assembly: AssemblyCompany("Microsoft")]
+[assembly: AssemblyProduct("Microsoft.WindowsAzure.ServiceLayer")]
+[assembly: AssemblyCopyright("Copyright © Microsoft 2012")]
@tg-msft
tg-msft added a note

Should both this line and the one two above use "Microsoft Corporation" instead?

@aliakb
aliakb added a note

This file was automatically generated and conveniently placed into a subdirectory, where it missed my attention. Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...soft.WindowsAzure.ServiceLayer/SerializationHelper.cs
((31 lines not shown))
+ static class SerializationHelper
+ {
+ /// <summary>
+ /// Deserializes the feed into a collection of items of the same type.
+ /// </summary>
+ /// <typeparam name="T">Type of result items in the collection</typeparam>
+ /// <param name="feed">Atom feed with serialized items</param>
+ /// <param name="itemAction">Additional action to perform on each item</param>
+ /// <returns>Collection of deserialized items</returns>
+ static internal IEnumerable<T> DeserializeCollection<T>(SyndicationFeed feed, Action<SyndicationItem, T> itemAction)
+ {
+ DataContractSerializer serializer = new DataContractSerializer(typeof(T));
+
+ foreach (SyndicationItem item in feed.Items)
+ {
+ yield return DeserializeItem(serializer, item, itemAction);
@tg-msft
tg-msft added a note

Nit - your foreach loop is the exact projection pattern embodied by LINQ's Select and you could shorten this up with return feed.Items.Select(item => DeserializeItem(serializer, item, itemAction);

@aliakb
aliakb added a note

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...soft.WindowsAzure.ServiceLayer/SerializationHelper.cs
((29 lines not shown))
+ /// Serialization/deserialization helper.
+ /// </summary>
+ static class SerializationHelper
+ {
+ /// <summary>
+ /// Deserializes the feed into a collection of items of the same type.
+ /// </summary>
+ /// <typeparam name="T">Type of result items in the collection</typeparam>
+ /// <param name="feed">Atom feed with serialized items</param>
+ /// <param name="itemAction">Additional action to perform on each item</param>
+ /// <returns>Collection of deserialized items</returns>
+ static internal IEnumerable<T> DeserializeCollection<T>(SyndicationFeed feed, Action<SyndicationItem, T> itemAction)
+ {
+ DataContractSerializer serializer = new DataContractSerializer(typeof(T));
+
+ foreach (SyndicationItem item in feed.Items)
@tg-msft
tg-msft added a note

Nit - I know this class isn't something users can access, but I usually still like to add parameter validation via Debug.Assert so it doesn't slow down production code but you make your constraints clear (and get more specific debugging help when things go sideways).

@aliakb
aliakb added a note

I agree. I am a big fan of asserts myself, and I usually use them for validating input parameters of internal methods. However, I never check such parameters for null values because using such parameters usually results in a NullReferenceException a few lines later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...soft.WindowsAzure.ServiceLayer/SerializationHelper.cs
((63 lines not shown))
+ /// Deserializes an atom item using given serializer.
+ /// </summary>
+ /// <typeparam name="T">Target object type</typeparam>
+ /// <param name="serializer">Serializer</param>
+ /// <param name="item">Atom item</param>
+ /// <param name="itemAction">Action to perform after deserialization</param>
+ /// <returns>Deserialized object</returns>
+ static T DeserializeItem<T>(DataContractSerializer serializer, SyndicationItem item, Action<SyndicationItem, T> itemAction)
+ {
+ string serializedString = item.Content.Xml.GetXml();
+
+ using (StringReader stringReader = new StringReader(serializedString))
+ using (XmlReader xmlReader = XmlReader.Create(stringReader))
+ {
+ T deserializedObject = (T)serializer.ReadObject(xmlReader);
+ itemAction(item, deserializedObject);
@tg-msft
tg-msft added a note

How do we handle failure? Do we let the underlying parse exception bubble up to the user?

@aliakb
aliakb added a note

Yes, they will get a deserialization exception.

@tg-msft
tg-msft added a note

I don't think they should - it'd be better if they received consistent exceptions so they don't need to worry about at AtomSerialiazationException today but a JsonSerializationException if we change the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...soft.WindowsAzure.ServiceLayer/SerializationHelper.cs
((85 lines not shown))
+ /// </summary>
+ /// <param name="item">Object to serialize</param>
+ /// <returns>Serialized representation</returns>
+ static internal string Serialize(object item)
+ {
+ // Serialize the content
+ string itemXml;
+
+ using (MemoryStream stream = new MemoryStream())
+ {
+ DataContractSerializer serializer = new DataContractSerializer(item.GetType());
+ serializer.WriteObject(stream, item);
+
+ stream.Flush();
+ stream.Seek(0, SeekOrigin.Begin);
+ using (StreamReader reader = new StreamReader(stream))
@tg-msft
tg-msft added a note

Will this cause a double Dispose on the stream? If you've turned on the latest version of FXCop it'll let you know either way on this one.

@aliakb
aliakb added a note

It will cause double Close, which does not cause problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...wsAzure.ServiceLayer/ServiceBus/IServiceBusService.cs
((18 lines not shown))
+using System.Threading.Tasks;
+
+using Windows.Foundation;
+
+namespace Microsoft.WindowsAzure.ServiceLayer.ServiceBus
+{
+ /// <summary>
+ /// Service bus service
+ /// </summary>
+ public interface IServiceBusService
+ {
+ /// <summary>
+ /// Lists all available queues in the namespace.
+ /// </summary>
+ /// <returns>Collection of queues</returns>
+ IAsyncOperation<IEnumerable<QueueInfo>> ListQueuesAsync();
@tg-msft
tg-msft added a note

You're going through a lot of work to abstract out the service into an interface so you have the flexibility to make changes down the road. Should we do the same for the QueueInfo type as well? Or do we expect that has different versioning characteristics than than the service itself?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tg-msft tg-msft commented on the diff
...oft.WindowsAzure.ServiceLayer/ServiceBus/QueueInfo.cs
((33 lines not shown))
+ public TimeSpan LockDuration { get; internal set; }
+
+ [DataMember(Order=1)]
+ public int MaxSizeInMegabytes { get; internal set; }
+
+ [DataMember(Order=2)]
+ public bool RequiresDuplicateDetection { get; internal set; }
+
+ [DataMember(Order=3)]
+ public bool RequiresSession { get; internal set; }
+
+ [DataMember(Order=4)]
+ public TimeSpan DefaultMessageTimeToLive { get; internal set; }
+
+ [DataMember(Order=5, Name="DeadLetteringOnMessageExpiration")]
+ public bool EnableDeadLetteringOnMessageExpiration { get; internal set; }
@tg-msft
tg-msft added a note

It's interesting that we're choosing to rename some of these parameters. Should we also rename things like Max -> Maximum or add Duration to the end of all our TimeSpan names for consistency?

@aliakb
aliakb added a note

I renamed some of these only to match member names with those specified in MSDN article on queue description. I was surprised to find out that the names used there do not match element names in the XML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...oft.WindowsAzure.ServiceLayer/ServiceBus/QueueInfo.cs
((67 lines not shown))
+
+ [IgnoreDataMember]
+ public Uri Uri { get; private set; }
+
+ /// <summary>
+ /// Constructor for serialization purposes.
+ /// </summary>
+ internal QueueInfo()
+ {
+ }
+
+ /// <summary>
+ /// Initializes the object after deserialization.
+ /// </summary>
+ /// <param name="item">Atom item</param>
+ internal void Initialize(SyndicationItem item)
@tg-msft
tg-msft added a note

What's the difference between your Initialize and a new .ctor?

@aliakb
aliakb added a note

The constructor is used by deserialization, and it cannot take any parameters. The Initialize method is called right after the object has been deserialized, and allows the object to get some extra parameters from the atom container it was enclosed in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Aliaksei Baturytski Some shared constants aa20ab2
...sAzure.ServiceLayer/ServiceBus/ServiceBusRestProxy.cs
((18 lines not shown))
+using System.Diagnostics;
+using System.IO;
+using System.Net.Http;
+using System.Text;
+using System.Threading.Tasks;
+
+using Windows.Data.Xml.Dom;
+using Windows.Foundation;
+using Windows.Web.Syndication;
+
+namespace Microsoft.WindowsAzure.ServiceLayer.ServiceBus
+{
+ /// <summary>
+ /// REST proxy for the service bus interface.
+ /// </summary>
+ class ServiceBusRestProxy: IServiceBusService
@tg-msft
tg-msft added a note

Is there any reason we're not just calling this ServiceBusService? It feels weird that you're changing from the word Service to the word Proxy.

@aliakb
aliakb added a note

There's already a public ServiceBusService class which is actually a factory for instantiating the service. Also, this is similar to what we did in Java SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tg-msft tg-msft commented on the diff
...sAzure.ServiceLayer/ServiceBus/ServiceBusRestProxy.cs
((42 lines not shown))
+ /// </summary>
+ HttpClient Channel { get; set; }
+
+
+ /// <summary>
+ /// Constructor.
+ /// </summary>
+ /// <param name="serviceOptions">Service options</param>
+ internal ServiceBusRestProxy(ServiceConfiguration serviceOptions)
+ {
+ Debug.Assert(serviceOptions != null);
+
+ ServiceConfig = serviceOptions;
+ HttpMessageHandler chain = new HttpErrorHandler(
+ new WrapAuthenticationHandler(serviceOptions));
+ Channel = new HttpClient(chain);
@tg-msft
tg-msft added a note

Are we not going to expose the pipeline so folks can add their own processing? What if they want to add retry logic, etc.?

@aliakb
aliakb added a note

Eventually, yes, but not in this checkin. I cannot expose .Net classes in WinMD libraries, so I need to come out with the right abstraction for the pipeline and filters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Aliaksei Bat... added some commits
Aliaksei Baturytski Updated project file bd5da9d
Aliaksei Baturytski Merge branch 'UnitTests' 8d52bd3
Aliaksei Baturytski Addressing code review issues 4bb3769
Aliaksei Baturytski Removing sensitive data 73cf42d
@tg-msft tg-msft commented on the diff
...ayer/Microsoft.WindowsAzure.ServiceLayer/Constants.cs
((14 lines not shown))
+//
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.ServiceLayer
+{
+ /// <summary>
+ /// Shared constants.
+ /// </summary>
+ internal static class Constants
+ {
+ internal const string ServiceBusServiceUri = "https://{0}.servicebus.windows.net/";
@tg-msft
tg-msft added a note

Should these be constants or resources? If they're resources, folks can change the service URI without recompiling the app. If the other libraries all have them as constants then I'm fine with that too as Azure won't likely be ever changing that URI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tg-msft tg-msft commented on the diff
...WindowsAzure.ServiceLayer/ServiceBus/QueueSettings.cs
((11 lines not shown))
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+using System;
+using System.Collections.Generic;
+using System.Runtime.Serialization;
+
+namespace Microsoft.WindowsAzure.ServiceLayer.ServiceBus
+{
+ /// <summary>
+ /// Queue creation options.
+ /// </summary>
+ [DataContract(Namespace="http://schemas.microsoft.com/netservices/2010/10/servicebus/connect", Name="QueueDescription")]
+ public sealed class QueueSettings
@tg-msft
tg-msft added a note

Why don't your DataContract and class have the same name?

@aliakb
aliakb added a note

For consistency with Java API - they call it QueueInfo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tg-msft tg-msft commented on the diff
....ServiceLayer/ServiceBus/WrapAuthenticationHandler.cs
((82 lines not shown))
+ if (token == null)
+ {
+ // Issue new authentication request.
+ Uri scopeUri = new Uri(ServiceConfig.ScopeHostUri, resourcePath);
+ HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Post, ServiceConfig.AuthenticationUri);
+ Dictionary<string, string> settings = new Dictionary<string, string>()
+ {
+ {"wrap_name", ServiceConfig.UserName},
+ {"wrap_password", ServiceConfig.Password},
+ {"wrap_scope", scopeUri.ToString()},
+ };
+
+ request.Headers.Accept.ParseAdd(Constants.WrapAuthenticationContentType);
+ request.Content = new FormUrlEncodedContent(settings);
+
+ HttpResponseMessage response = Channel.SendAsync(request).Result;
@tg-msft
tg-msft added a note

Do you need to wait for the SendAsync to complete?

@aliakb
aliakb added a note

Accessing Task.Result property waits for completion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...oft.WindowsAzure.ServiceLayer/ServiceBus/WrapToken.cs
((16 lines not shown))
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Globalization;
+using System.Linq;
+using System.Net;
+using System.Net.Http;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.ServiceLayer.ServiceBus
+{
+ /// <summary>
+ /// WRAP token; used for authenticating outgoing web requests.
+ /// </summary>
+ class WrapToken
@tg-msft
tg-msft added a note

Missing an access modifier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@aliakb

The request is obsolete; closing it.

@aliakb aliakb closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 21, 2012
  1. Initial checkin for WinMD API

    Aliaksei Baturytski authored
Commits on Feb 22, 2012
  1. Unit tests for queue management

    Aliaksei Baturytski authored
  2. Updated file headers

    Aliaksei Baturytski authored
  3. Explicit visibility on classes and their members

    Aliaksei Baturytski authored
  4. Enclosing single-line statements into curlies

    Aliaksei Baturytski authored
  5. Punctuation in comments

    Aliaksei Baturytski authored
  6. Some shared constants

    Aliaksei Baturytski authored
  7. Updated project file

    Aliaksei Baturytski authored
  8. Merge branch 'UnitTests'

    Aliaksei Baturytski authored
  9. Addressing code review issues

    Aliaksei Baturytski authored
  10. Removing sensitive data

    Aliaksei Baturytski authored
  11. Fixes for code analysis issues

    Aliaksei Baturytski authored
Something went wrong with that request. Please try again.