gazuntype / graphQL-client-unity

This repository houses a unitypackage that can be included in your Unity Project to enable it communicate with a graphQL server.
Apache License 2.0
292 stars 50 forks source link

JsonToArgument function fails with valid input #8

Open sporeservant opened 4 years ago

sporeservant commented 4 years ago

I'm not 100% sure what is going on here, but I have been debugging for quite a while - was hoping to be able to submit a PR, but I hit a deadend.

Using SetArgs(object o) function, the Json conversation works fine (even with Enum), but then when JsonToArgument runs, and the following error happens:

System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index

Based on the debugger, it seems that indexes has a Length(Count) of 1 here:

        if (jsonChar[i] == ':'){
            jsonChar[indexes[0]] = ' ';
            jsonChar[indexes[1]] = ' ';
        }

but it isn't clear exactly why. I added a check to make sure index[1] exists before running that second call to jsonChar[indexes[1]], but it results in a malformed query.

I made an isolated example on Repl.it if it is any help: https://repl.it/repls/AbleVerticalMaps

The input to the function is a direct copy of what is received after string json = JsonConvert.SerializeObject(inputObject, new EnumInputConverter());. In my schema I have an enum like AssetType:

enum AssetType {
  DRESSING
  GLOBAL
  MAP
  PLAYERHANDOUT
  TOKEN
  OTHER
}

and then in my local code:

enum AssetType {
  DRESSING
  GLOBAL
  MAP
  PLAYERHANDOUT
  TOKEN
  OTHER
}

I thought the enum might be the problem, because I have other queries that do work just fine. But I don't think AssetType enum is the problem here.

Code from Repl.it if it helps:

using System;
using System.Collections;
using System.Collections.Generic;

class MainClass {
  private static string JsonToArgument(string jsonInput){
    char[] jsonChar = jsonInput.ToCharArray();
    List<int> indexes = new List<int>();
    jsonChar[0] = ' ';
    jsonChar[jsonChar.Length - 1] = ' ';
    for (int i = 0; i < jsonChar.Length; i++){
        if (jsonChar[i] == '\"'){
            if (indexes.Count == 2)
                indexes = new List<int>();
            indexes.Add(i);
        }

        if (jsonChar[i] == ':'){
            jsonChar[indexes[0]] = ' ';
            jsonChar[indexes[1]] = ' ';
        }
    }

    string result = new string(jsonChar);
    return result;
  }

  public static void Main (string[] args) {
    JsonToArgument("{\"input\":{\"adventureId\":null,\"assetSubType\":null,\"assetType\":Map,\"campaignId\":null,\"contentPackageId\":null,\"description\":\"A wonderful place to go!\",\"encounterId\":null,\"name\":\"Abandoned House\",\"originalImageUrl\":\"https://i0.wp.com/www.fantasticmaps.com/wp-content/uploads/2010/06/Attachment-1.jpeg\",\"s3ImageUrl\":\"https://omitted.s3.amazonaws.com/08d7fa90-a716-4083-8fd6-41d21e317fe3/Attachment-1.47c282ef-8c42-4c29-8321-80360a2eaa36.jpeg\",\"userId\":\"08d7fa90-a716-4083-8fd6-41d21e317fe3\"}}");
  }
}
sporeservant commented 4 years ago

I believe that the issue is something to do with the URL strings. It seems if you remove the URL strings, the function will run correctly. Maybe the URL needs to be escaped in some why before being sent in? If you change it to:

    JsonToArgument("{\"input\":{\"description\":\"A wonderful place to die!\",\"name\":\"Abandoned House\",\"userId\":\"08d7fa90-a716-4083-8fd6-41d21e317fe3\"}}");

The function stops failing to parse indexes.

sporeservant commented 4 years ago

Oh, I see now. When value is like "http://" or "https://", it is catching that semi-colon and treating it as though it is part of the parseable input.

In my case, I can just omit the protocol (http/https) but I do think this function will need to be made more sophisticated to ensure that the ":" colon it is matching on is legitimate and not actually part of the string value.

gazuntype commented 4 years ago

hi, i apologize for just seeing your issue.

also, thank you for pointing it out to me and doing proper debugging. i understand the problem clearly because of you and it's obvious i didn't consider this edge case. are you working on a PR to fix it or should i take it into my own hands?

netpoetica commented 4 years ago

Hello, thanks for getting back to me (side note: noticed that I am on my other account now, but I am same user as sporeservant)

I actually spent some time trying to solve this but I just couldn't quite figure out the internals of this parsing engine and how to fix it. I am not sure that it is a good long-term solution to do something like "Look for protocol matches when you see : like file://,http://,https://,ws:// and skip if you do" - this seems a bit amateur to me. I think the wiser option is to modify that function I mentioned above to do some different lexical parsing, like

WHEN find( colon, i.e. : ), parseUntilNext( next escaped open quote i.e. /" )

But if I am being honest, I am not really sure what your code there in that function is doing and C# is not my primary language (although admittedly, that particular function isn't really doing anything language-specific). If this is something you have a clear idea how to implement, it will be more reliable in your hands. Also not to sound pushy, but if you could grab a pot of coffee and sit down and just go through the code adding comments, I think it would go a long way to getting some contributions :) the only comments in the whole source are ToDos

Also, as a side note: I am trying to create a GUI-free update to this project because the GUI is just not reliable or intuitive (it is very good for free :D). I have this operational now but it is quite hacky. I am working tireless hours on a student project for my grad degree right now so I am struggling to make this a priority, but it is on my mind and I hope to get something together for this.

PulsarFox commented 3 years ago

Any news on this ? I tried to use JsonB parameters and I have the same issue