bugthesystem / FireSharp

An asynchronous cross-platform .Net library for Firebase
The Unlicense
696 stars 147 forks source link

Bug in temporary cache results in malformed tree #66

Open cdiddy77 opened 8 years ago

cdiddy77 commented 8 years ago

I think this is probably causing #62 among others. Paste the below code into your FiresharpTests and see the failure. When you parse the update from the server, you incorrectly construct the SimpleCacheItem tree. The first member of the collection contains the second member, contains the third, etc. Not sure why that results in 4 notifications

            [Test, Category("INTEGRATION"), Category("ASYNC")]
    public async void PushAndListenAsync()
    {
        var todos = new[]
        {
            new Todo { name = "Execute PUSH4", priority = 1 },
            new Todo { name = "Execute PUSH4 twice", priority = 2 },
        };

        var response = await _client.PushAsync("todos/push/pushAndListenAsync", todos[0]);
        Assert.NotNull(response);
        Assert.NotNull(response.Result);
        Assert.NotNull(response.Result.Name); /*Returns pushed data name like -J8LR7PDCdz_i9H41kf7*/
        Console.WriteLine(response.Result.Name);
        response = await _client.PushAsync("todos/push/pushAndListenAsync", todos[1]);
        Assert.NotNull(response);
        Assert.NotNull(response.Result);
        Assert.NotNull(response.Result.Name); /*Returns pushed data name like -J8LR7PDCdz_i9H41kf7*/
        Console.WriteLine(response.Result.Name);

        int onTodosCount = 0;

        var listenResponse = _client.OnAsync("todos/push/pushAndListenAsync",
            //added
            (sender, args, context) =>
            {
                Console.WriteLine(args.ToJson());
                Interlocked.Increment(ref onTodosCount);
            },
            //changed
            (sender, args, context) =>
            {
                Console.WriteLine(args.ToJson());
                Assert.Fail("nothing should have changed yet, only added");
            },
            // removed
            (sender, args, context) =>
            {
                Console.WriteLine(args.ToJson());
                Assert.Fail("nothing should have been removed, only added");
            });

        await Task.Delay(4000);
        Assert.AreEqual(2, onTodosCount);
    }
4nthonylin commented 8 years ago

The reason why the cache is being incorrectly constructed is because in the switch statement on line 86 in "TemporaryCache.cs", there is no case for JsonToken.EndOfObject. To fix all you have to do is add: case JsonToken.EndOfObject: break;

As for why the eventhandler is fired four times is because the eventhandler is called for every single value. Your todo object contains two values hence for each todo the eventhandler is fired twice resulting in 4 notifications. Currently figuring out a way to rewrite the cache to fire notifications per object.

kingsurf commented 7 years ago

Can we please get this bug fix into the master branch? I just spent an hour fixing this issue.

deebash commented 5 years ago

This commit https://github.com/AButler/FireSharp/commit/c405f9635c03bc8cd16e6a115257a8b7ff06c447 fixes the issue. But NuGet package won't have this fix. Please do change as in the commit to TemporaryChange.cs