keenlabs / keen-sdk-net

A .NET SDK for the Keen IO API
MIT License
37 stars 24 forks source link

Handle JSON trimming better #118

Open masojus opened 6 years ago

masojus commented 6 years ago

We should trim in some places when going over the wire, and based on a flag or in DEBUG or while running tests or some combination, pretty-print. We should also not compare against raw strings in tests, but rather build up a JSON object and stringify before comparing--again with or without pretty-printing based on a flag or the config or something. There are also potential issues with URL query strings in terms of format and length.

Most recently embedding "\r\n" in tests caused failures on non-Windows platforms.

Here are my notes from a few months back:

    - The fact that we don't trim out JSON strings makes for garbage URLs when performing queries:
        ? Here's a sample query string with a not_contains filter: "?event_collection=testCollection&timeframe=this_month&filters=%5B%0D%0A%20%20%7B%0D%0A%20%20%20%20%22property_name%22%3A%20%22propertyName%22%2C%0D%0A%20%20%20%20%22operator%22%3A%20%22not_contains%22%2C%0D%0A%20%20%20%20%22property_value%22%3A%20%22four%22%0D%0A%20%20%7D%0D%0A%5D"  
        ? Which comes from this: 
        {[filters, [
          {
            "property_name": "propertyName",
            "operator": "not_contains",
            "property_value": "four"
          }
        ]]}
        ? Most of it is "\r\n" and whitespace, which wastes precious characters in the URL and also looks gross and uses unnecessary bits of bandwidth.
        ? Look at parms.Add(KeenConstants.QueryParmFilters, filters == null ? "" : JArray.FromObject(filters).ToString());
        ? We should instead call .ToString(Newtonsoft.Json.Formatting.None)
        ? For that matter, does the way we format lists of filters actually even work? Seems to, but instead of doing &filter=…&filter=…, we use EscapeDataString(..the Jarray toString() representation…) which ends up being &filters=%5B….%5D  which is the encoding for […] so it's literally escaping the JSON string for the array. What does curl do? It seems like python requests.get() (according to their docs) should do something different. So does the back end just do something smart and detect either format? Or is one of them wrong?
        ? Actually, the Python Requests docs show that if you pass "params" as an object, a list gets turned into multiple query params with the same key. However, Keen's SDK, in get_params() in client.py, turns a group_by of type "list" into a string by using json.dumps(), which seems to indicate that then that string will get url-encoded the way the .NET SDK does by using EscapeDataString() for filters…maybe. So should the same be done when turning group_by into a list?