Added HTTP option for the platforms that does not support Web Socket & Handle null aggregate keys.

[adnan-khan]

No description provided.

[colethecoder]

Hi thanks for this, in general I think it looks good and is a useful feature to add. I have a performance branch that probably clashes with this a bit so I will review with that and see if we need any changes. It's going to be next week before I get chance to look at this properly though.

[adnan-khan]

@colethecoder Did you get a chance to look at this again ?

[colethecoder]

@adnan-khan sorry, I have looked but just not had time to merge. The only thing that jumped out at me is that we now have EventWebSocketRepository for example that can then do a non-web socket request. It feels like all these instances should be renamed to something like EventApiRepository to avoid confusion. What do you think?

[colethecoder]

I haven't merged this yet because I'm not sure selecting the transport type when constructing the Environment is the best idea. The problem this brings is that if you get a list of environments using GetEnvironmentAsync or GetEnvironmentsAsync on the Connection object you are not going to be able to select the transport type.

I think, perhaps, it would be better to select the transport type when a query is executed. This however will involve a bit of refactoring. I'll look into it this week.

[colethecoder]

Hi, I have revisited this and I think there a few things that would need to be implemented to make this viable as a feature. The transport mode would need to be configured on the execution i.e.

.ExecuteAsync(); would be web sockets to avoid breaking changes

.ExecuteAsync(Https) would be Https

The query objects would need to be able to query WebSockets or HTTP, I think we could probably make the repositories totally static and then pass two Funcs into the query constructor (these can then be the proper implementations normally or mock versions for testing).

There are a load of references to webSocketRepository now that might be Http which need cleaning up.

The CancellationToken is totally ignored via Http which is wrong. I think you could build this into a web request with an extension method something like (untested):

        public static async Task<HttpWebResponse> GetResponseAsync(this HttpWebRequest request, CancellationToken ct)
        {
            using (ct.Register(() => request.Abort(), useSynchronizationContext: false))
            {
                try
                {
                    var response = await request.GetResponseAsync();
                    return (HttpWebResponse)response;
                }
                catch (WebException ex)
                {
                    if (ct.IsCancellationRequested)
                    {
                        throw new OperationCanceledException(ex.Message, ex, ct);
                    }
                    throw;
                }
            }
        }

The web socket repo currently checks for errors and converts to bespoke Chronological exceptions using IErrorToExceptionConverter, the Http one would need to do this too.

The Http repo does this at the moment:
var httpQuery = JToken.Parse(query)["content"].ToString();
I get why this is happening because the existing version builds the headers into the query but now it is creating a JObject for the query, turning it into a string, parsing back to a JToken selecting just the content and then writing back out to a string, this is really inefficient and ugly. It needs refactoring so that only the correct Json is built for the query.

I started trying to implement all this but I don't have a great deal of time at the moment so don't expect anything soon. I'm happy to discuss and provide guidance if you want to attempt the changes. It is worth noting that there is a brand new REST API with the new TSI preview (https://docs.microsoft.com/en-us/rest/api/time-series-insights/preview-query) so perhaps effort is better spent building a new Chronological for that rather than implementing a lot of changes on a version that may be outdated quite soon.