Add orderBy* comparators.

[jamestalmage]

Comparators for ordering refs according to the spec. Should be comprehensive.

[bendrucker]

I'm going to just leave this open until I'm ready to work on shipping 2.0 queries

[JustinTulloss]

My thoughts on this is that @jamestalmage has already gone too complex. The object of this library should be to provide a comprehensive mock, not exactly emulate its behavior. I would expect orderBy* functions to just return the data they contain, I'm not sure what we gain by actually implementing the ordering.

[bendrucker]

I'm not sure what we gain by actually implementing the ordering

The ability to test any code that depends on the order of data events.

As mentioned above, the only reason this is un-merged is because the query code is not well suited to the 2.0 queries and really need to be written from scratch. I just don't have the bandwidth to tackle that right now as a volunteer. I'd be all for a simpler implementation for mocking 2.0 queries. But I don't see a way to accomplish that short of re-engineering the implementation.

[JustinTulloss]

You test the UI response to order by reordering the data on the test side. Reality is defined by the tester, not the mock. In my opinion it makes the mock easier and the tests more obvious. 

[bendrucker]

How do you intend to reorder the data on the test side?

[JustinTulloss]

I wouldn't. I don't think it needs to be done, you can always just specify new data and make sure your code reacts properly to new data. The order being correct is something we trust firebase to do, not something that's actually inherently important to the test.

On Mon Feb 16 2015 at 4:04:11 AM Ben Drucker notifications@github.com wrote:

How do you intend to reorder the data on the test side?

— Reply to this email directly or view it on GitHub https://github.com/katowulf/mockfirebase/pull/61#issuecomment-74498906.

[jamestalmage]

Reality is defined by the tester, not the mock.

If the testers reality doesn't match actual reality, well those tests are not going to be terribly useful.

The order being correct is something we trust firebase to do, not something that's actually inherently important to the test.

Yes, we trust firebase to order the data correctly, so why trust ourselves to order input data correctly for every single test we write? The ordering has wide ranging impact; Basically every event that is fired has a dependency on how data is ordered. child_added events are called according to query order. For limitToXXX queries you will also see paired child_removed, child_added as one child moves within the query bounds and "bumps" another out. Also the previousKey passed to child_xxx events is based on ordering.

Say you've got a Firebase reference ref pointing to server data like this:

{ "a" : "foo"
, "c" : "bar"  
, "d" : "baz"
}

If you create a query and register events as follows:

var query = ref.orderByKey().limitToFirst(2);
query.on('child_added', addedCallback);
query.on('child_removed', removedCallback);
query.on('value', valueCallback);

Based on the initial data, your listeners are going to get called as follows (guaranteed in this order):

1. addedCallback(addSnap, prevKey) where prevKey === null, .key() === 'a', and .val() === 'foo'
2. addedCallback(addSnap, prevKey) where prevKey === 'a', .key() === 'c', and .val() === 'bar'
3. valueCallback(valueSnap) where valueSnap.val() === { a: 'foo', c: 'bar' }

Later, lets say you add a new child to backing data:

ref.child("b").set("qux");

This one action will trigger the following events (guaranteed in this order):

1. removeCallback(removeSnap) where prevKey === 'a',.key() === 'c', and .val() === 'bar'
2. addedCallback(addSnap) where prevKey === 'a',.key() === 'b' and .val() === 'qux'
3. valueCallback(valueSnap) where valueSnap.val() === { a: 'foo', b: 'qux' }

Also, valueSnap.forEach(cb) will call cb according to the ordering specified.

Sure, you could figure all this behavior out and use something like sinon to fire all the appropriate callbacks in the right order (and make sure your stubs for snap.forEach iterate in the correct order). That is a lot of work per test, and prone to error.

[JustinTulloss]

Pretty much agree with you @jamestalmage. I think the callbacks should all fire in a way that resembles real life since that's what you're testing. But actually reordering the data doesn't seem all that important to me.

I would be fine writing a test that was something along the lines of (pseudocode):

ref = setupDummyData();
render(); // Let's say we're testing a UI
simulateReorderEvent('name');
assert(ref.isOrderedByChild('name'));
ref.push(newData);
assertFirstChildRemoved();
assertNewLastChildExists();

The important bits:

• We can figure out if our sort is correct without looking at the data, we can just ask the mock what the current ordering parameters are.
• We can assert that the program is reacting appropriately to the underlying changes in data without necessarily changing the data.

But this is all a rather unimportant argument, if you want to sort the data by all means go ahead. I just think it would be faster (development wise) and just as effective to just stub out the state machine. I am lazy, you see.

[jamestalmage]

@JustinTulloss I think you are confused as to how the API works. Firebase ref's are immutable, calling ref.orderByXXX does not change the ordering of the backing data at all, but returns a new view of the data, that will receive events from the backing data ordered in the way you've chosen.

This is totally legit:

ref.orderByKey().limitToFirst(10).on('value', listener1);
ref.orderByKey().limitToLast(10).on('value', listener2);

Assuming that ref points to a location with children >= 20, the two listeners will receive snapshots containing completely disjoint datasets (no two children with the same key between them).

Similarly, this is totally legit as well.

ref.orderByKey().on('child_added', listener3);
ref.orderByValue().on('child_added', listener4);

The two listeners will receive events in the order specified on a per listener basis.

In other words, listener1 is always going to be called with the first 10 children orderedByKey subsequent calls to ref.orderByXXX are not going to change that.

Ordering is not specified when you set data to firebase, but when you read it. So the api you recommend does not make a lot of sense

simulateReorderEvent('name'); 
assert(ref.isOrderedByChild('name')); 
// ref's are not ordered, your query is and that ordering is immutable

[bendrucker]

@jamestalmage Thanks for going so in depth on this. This thread is a great resource. I'm going to unsubscribe and let you guys continue if you'd like. Ping me by username if you need me James.

Just to be ultra clear, the query API is going to continue to reproduce Firebase's behavior per this PR.

[katowulf]

@JustinTulloss thanks for the great ideas and for the feedback. Really appreciated. It will certainly help to shape the approach we take.

It's possible to achieve similar behavior to some of the things you've described using spies. For example, I could implement orderByChild() as follows (assumes Jasmine 2.0; mocha and 1.x have similar constructs):

var fb = new MockFirebase();
spyOn(fb, 'orderByChild').and.callFake(function(chlidKey) {
    return fb; // do nothing to data ordering, but allow orderByChild() to be called
});

I utilize this approach often when I want the data to return a specific value or result. I could even return an artificial list of pre-sorted results, or whatever needs my test has. I utilize this approach often when the exact data and behavior are well known.

To wax a bit intellectual, MockFirebase tries to tackle some of the more intricate and convoluted aspects of testing with Firebase that are not quite as straightforward.

Cheers, Kato

[JustinTulloss]

@jamestalmage I definitely understand how the library works, I was asking for you to follow my logic not read the code literally. I should have been more clear, I'm simulating a reorder in the UI, which has the side effect of performing a query on firebase. A perfect use case for a mock.

Totally agree that the events should fire in the expected order. However, I can see that you don't agree with my approach so I won't be updating this PR. Looking forward to seeing something like this merged eventually.

@katowulf Yes, that's basically what I do now. The problem (and what this pull request will eventually solve) is that the events don't trigger in the proper order when you just set up a series of spies. I do think something like this is needed.

[doowb]

@bendrucker @katowulf what's the status on this?

I just started using mockfirebase and ran into orderByChild is not a function. I can try using sinon with the mockfirebase reference created, but will that work if orderByChild isn't even a function?