Skip to content

V3: orderByChild + startAt or endAt #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
florianbepunkt opened this issue Dec 1, 2016 · 36 comments
Closed

V3: orderByChild + startAt or endAt #174

florianbepunkt opened this issue Dec 1, 2016 · 36 comments

Comments

@florianbepunkt
Copy link

@Salakar Sorry to bother you again. It seems like orderByChild is not working, at least when used with startAt or endAt. I have a list of message objects that I want to query based on their timestamp.

This code returns zero results

firestack
  .database()
  .ref('messages/-KXlqrDgbJg_-FJCoDfE/')
  .orderByChild('createdAt')
  .startAt(1480609558242)
  .once('value')
  .then((snapshot) => {
    console.log('my snap', snapshot);
  })

while this code returns all results

firestack
  .database()
  .ref('messages/-KXlqrDgbJg_-FJCoDfE/')
  .orderByChild('createdAt')
  .endAt(1480609558242)
  .once('value')
  .then((snapshot) => {
    console.log('my snap', snapshot);
  })

Based on my sample data below…both results are wrong. The first example should return the first last two objects, the second example should return the first two objects.

I tried both the v3 branch of this repo as well as master branch of your fork. Based on the sample data the query above should return the first object. But nothing is returned


{
  "-KXlqrDgbJg_-FJCoDfE" : {
    "-KXv8Uvyix9Ns5FeF1O1" : {
      "_id" : "-KXv8Uvyix9Ns5FeF1O1",
      "createdAt" : 1480608580717,
      "text" : "Test123",
      "user" : {
        "_id" : "If2AZYNhruXa4ELtYoviNe7JS2S2"
      },
      "wasSeen" : false
    },
    "-KXvCCjPLePNohIkjSw3" : {
      "_id" : "-KXvCCjPLePNohIkjSw3",
      "createdAt" : 1480609553540,
      "text" : "Test456",
      "user" : {
        "_id" : "If2AZYNhruXa4ELtYoviNe7JS2S2"
      },
      "wasSeen" : false
    },
    "-KXvCDsZq__24IkB6zGs" : {
      "_id" : "-KXvCDsZq__24IkB6zGs",
      "createdAt" : 1480609558242,
      "text" : "Test789",
      "user" : {
        "_id" : "If2AZYNhruXa4ELtYoviNe7JS2S2"
      },
      "wasSeen" : false
    },
    "-KXvQisiS1bsm3oYCdll" : {
      "_id" : "-KXvQisiS1bsm3oYCdll",
      "createdAt" : 1480613359327,
      "text" : "Test101112",
      "user" : {
        "_id" : "If2AZYNhruXa4ELtYoviNe7JS2S2"
      },
      "wasSeen" : false
    }
  }
}
@florianbepunkt florianbepunkt changed the title V3: orderByChild V3: orderByChild + startAt or endAt Dec 1, 2016
@Salakar
Copy link
Collaborator

Salakar commented Dec 2, 2016

Hmm weird one @florianbepunkt - database in the current pushed up state of my branch has not been touched bar a PR from @BlooJeans which was on master that I manually pulled in.

Will look at this today and get back to you, the example helps, cheers!

@florianbepunkt
Copy link
Author

@Salakar Maybe this is of interest #82 – it was not working properly on master until recently

@Salakar
Copy link
Collaborator

Salakar commented Dec 2, 2016

@florianbepunkt looks like @chrisbianca fixed this issue 3 days ago on my branch - have you tried pulling the latest?

This commit:
Salakar@ec9f488

@florianbepunkt
Copy link
Author

@Salakar I did. To make sure I just installed it again via npm install --save https://github.com/Salakar/react-native-firestack.git

The commit only changes things on android side… I see this problem with iOS.

@chrisbianca
Copy link
Contributor

I only updated the Android side as that's where I'd noticed the issue.
Just taken a look and a similar sort of change does need to happen on the iOS side - I'll see if I can take a look in a little while...

@Salakar
Copy link
Collaborator

Salakar commented Dec 2, 2016

@florianbepunkt oh ok my bad, thought you were having an android issue sorry.

@chrisbianca that would be awesome thanks!

@florianbepunkt
Copy link
Author

I'm not sure if this is related… using a fresh install from Salakar's fork I see the same issue on android.

Using my sample data


firestack
  .database()
  .ref('messages/-KXlqrDgbJg_-FJCoDfE/')
  .orderByChild('createdAt')
  .startAt(1480609558242)
  .once('value')
  .then((snapshot) => {
    console.log('my snap', snapshot);
  })

returns zero values…

@chrisbianca
Copy link
Contributor

Ok, I don't have any data setup to test this with at the moment.
When I get some time later on, I'll take a look and see if I can figure out what's going on.

@florianbepunkt
Copy link
Author

@chrisbianca okay. I throw together a simple test project and upload it if that helps

@chrisbianca
Copy link
Contributor

@florianbepunkt that would be great!

@florianbepunkt
Copy link
Author

@chrisbianca @Salakar Okay, here we go… https://github.com/florianbepunkt/fireStackQuery

you can clone the repo, run npm install and run it. I've tested it on ios and it reproduces the issue.

@chrisbianca
Copy link
Contributor

@florianbepunkt @Salakar

So I've figured out why it's broken in Java... It's passing the startAt and endAt as a String rather than a number. The fix is an awful lot of repetitive code but I can't think of a better way to work around it! Just pushed the fix to the v3 branch and sorted for boolean types too.

Will try and look at iOS now...

@florianbepunkt
Copy link
Author

@chrisbianca great! just checked android version with your commit

@chrisbianca
Copy link
Contributor

I've just committed the equivalent fix for iOS. I rewrote a large part of the iOS database module to bring it more in line with Android and stop regenerating firebase references unnecessarily. Let me know if you have any other issues...

@florianbepunkt
Copy link
Author

@chrisbianca I just freshly cloned my example project linked above and run npm install… tne ios part won't boot. android works fine.

this is the output of react-native log-ios

Dec  3 11:43:04 MacBook-Pro fireStackQuery[450] <Error>: [] nw_host_stats_add_src recv too small, received 24, expected 28
Dec  3 11:43:04 MacBook-Pro fireStackQuery[450] <Notice>: Initializing <RCTBatchedBridge: 0x6000001993d0> (parent: <RCTBridge: 0x6000000ae820>, executor: RCTJSCExecutor)
Dec  3 11:43:04 MacBook-Pro fireStackQuery[450] <Error>: [] nw_host_stats_add_src recv too small, received 24, expected 28
Dec  3 11:43:04 MacBook-Pro fireStackQuery[450] <Warning>: Setting up Firestace instance
Dec  3 11:43:04 MacBook-Pro fireStackQuery[450] <Error>: [Firebase/Core][I-COR000003] The default Firebase app has not yet been configured. Add [FIRApp configure] to your application initialization. Read more: https://goo.gl/ctyzm8.
Dec  3 11:43:04 MacBook-Pro fireStackQuery[450] <Error>: *** Terminating app due to uncaught exception 'FIRAppNotConfigured', reason: 'Failed to get default FIRDatabase instance. Must call FIRApp.configure() before using FIRDatabase.'
        *** First throw call stack:
        (
                0   CoreFoundation                      0x000000010631434b __exceptionPreprocess + 171
                1   libobjc.A.dylib                     0x000000010585e21e objc_exception_throw + 48
                2   CoreFoundation                      0x000000010637d265 +[NSException raise:format:] + 197
                3   fireStackQuery                      0x0000000103c4654f +[FIRDatabase database] + 79
                4   fireStackQuery                      0x0000000103e7af65 -[FirestackDatabase init] + 117
                5   fireStackQuery                      0x0000000103d9a798 -[RCTModuleData setUpInstanceAndBridge] + 2056
                6   fireStackQuery                      0x0000000103d9c286 __25-[RCTModuleData instance]_block_invoke + 38
                7   fireStackQuery                      0x0000000103dbba24 RCTExecuteOnMainThread + 84
                8   fireStackQuery                      0x0000000103d9bf9d -[RCTModuleData instance] + 749
                9   fireStackQuery                      0x0000000103d8c1fc __52-[RCTBatchedBridge prepareModulesWithDispatchGroup:]_block_invoke + 124
                10  libdispatch.dylib                   0x000000010a09d810 _dispatch_call_block_and_release + 12
                11  libdispatch.dylib                   0x000000010a0bf12e _dispatch_client_callout + 8
                12  libdispatch.dylib                   0x000000010a0a62bc _dispatch_main_queue_callback_4CF + 429
                13  CoreFoundation                      0x00000001062d84f9 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
                14  CoreFoundation                      0x000000010629df8d __CFRunLoopRun + 2205
                15  CoreFoundation                      0x000000010629d494 CFRunLoopRunSpecific + 420
                16  GraphicsServices                    0x000000010ab90a6f GSEventRunModal + 161
                17  UIKit                               0x00000001086f2964 UIApplicationMain + 159
                18  fireStackQuery                      0x0000000103b4b87f main + 111
                19  libdyld.dylib                       0x000000010a10868d start + 1
        )
Dec  3 11:43:04 MacBook-Pro SpringBoard[98560] <Error>: [KeyboardArbiter] HW kbd: Failed to set (null) as keyboard focus
Dec  3 11:43:04 MacBook-Pro com.apple.CoreSimulator.SimDevice.DB5AF926-27C5-4344-8273-3A5318DEE307.launchd_sim[98543] (UIKitApplication:org.reactjs.native.example.fireStackQuery[0xf773][450]) <Notice>: Service exited due to Abort trap: 6
Dec  3 11:43:04 MacBook-Pro assertiond[98564] <Warning>: notify_suspend_pid() failed with error 7
Dec  3 11:44:23 MacBook-Pro CoreSimulatorBridge[98569] <Warning>: Pasteboard change listener callback port <NSMachPort: 0x7f8983102810> registered

@chrisbianca
Copy link
Contributor

@florianbepunkt Ah yes, for the time being, add the following in your AppDelegate.m:

At the top:

#import <Firebase.h>

In didFinishLaunchingWithOptions:

[FIRApp configure];

I'll look at why Firebase isn't being initialised before the database later today / tomorrow

@florianbepunkt
Copy link
Author

@chrisbianca Thank you. This did the trick

@chrisbianca
Copy link
Contributor

@florianbepunkt Just pushed a change so you shouldn't need to configure your app in the iOS code anymore

@florianbepunkt
Copy link
Author

@chrisbianca First of all thank you. I feel a bit lost here… in my demo example I linked above, this works. When I put it in my app I'm working on, startAt and endAt have no effect. I made sure I updated firestack… it's up to date. and it's the same data set.

any idea how this can happen?

@florianbepunkt
Copy link
Author

@chrisbianca okay, got it working now. I had to delete the ios build folder. either I messed up my xcode project settings or when you run the app it's not built from scratch… anyway it works now 👍

I found a small error by accident … I'm working on a kind of messenger. so when I open a conversation I attach a listener for messages, when I leave the conversation I detach the listener by calling

firestack
      .database()
      .ref('messages')
      .child(connversationID)
      .off();

by accident I called the code above without attaching a listener first...which throws the error below. has the method for detaching a listener changed? anyways for me it's no issue, just wanted to let you know.

Exception '*** -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[3]' was thrown while invoking off on target FirestackDatabase with params (
    "/messages/-KXlqrDgbJg_-FJCoDfE",
    "",
    "",
    33
)

@chrisbianca
Copy link
Contributor

@florianbepunkt I haven't changed anything around the off() functionality so this may be a pre-existing issue.

I believe @Salakar is going to look at the JS side of the database and try and simplify the listeners so may cover this then.

@florianbepunkt
Copy link
Author

Okay. I did not notice this error before the update.… i did some more testing. I also get this error when I properly atatched a listener first… so basically whenever I remove a listener.

@BlooJeans
Copy link
Contributor

BlooJeans commented Dec 5, 2016

@florianbepunkt Try:

let conversationRef = firestack.database().ref('messages').child(connversationID);
conversationRef.on('child_added', (data) => console.log(data));
conversationRef.off('child_added');

Everytime you modify the reference (adding modifiers or changing the path), it effectively creates a new unique reference. Two references that have the same 'builder' are not the same internally.

@florianbepunkt
Copy link
Author

@BlooJeans Thanks for the clarification. Makes sense.

@Salakar
Copy link
Collaborator

Salakar commented Dec 6, 2016

@florianbepunkt @chrisbianca - @Ehesp and I are having the same issue at the moment:

firestack
    .database()
    .ref('matches')
    .orderByChild('startsAt')
    .startAt(1480982400)
    .endAt(1481068799)
    .on('value', snap => console.log(snap.val()));

Returns no results on firestack. The exact same query however on the web sdk returns correctly and gives matches all for that time range.

We're using the head of my fork which includes your changes @chrisbianca - am i missing something?

A possibly better way to handle the value types would be get the types on the JS side i.e. it gets sent over as startsAt:number:1234567890 then we don't need to do any guess work on either ios / android - thoughts?

Sorry for the slow responses also, have been ill, urgh.

@Salakar Salakar reopened this Dec 6, 2016
@florianbepunkt
Copy link
Author

@Salakar I can confirm (sorry @chrisbianca didn't notice since I had no need to combine startAt and endAt yet)

although I get results, when I combine startAt and endAt

both queries below return the same results. updated my test project https://github.com/florianbepunkt/fireStackQuery

firestack
      .database()
      .ref('messages/-KXlqrDgbJg_-FJCoDfE/')
      .orderByChild('createdAt')
      .endAt(1480609558242)
      .once('value')
      .then((snapshot) => {
        console.log('queryTest orderByChild endAt test > snapshot', snapshot);
      })

    firestack
      .database()
      .ref('messages/-KXlqrDgbJg_-FJCoDfE/')
      .orderByChild('createdAt')
      .startAt(1480609558242)
      .endAt(1480617793311)
      .once('value')
      .then((snapshot) => {
        console.log('queryTest orderByChild startAt and endAt test > snapshot', snapshot);
      })

@chrisbianca
Copy link
Contributor

@florianbepunkt thanks, we've been discussing off this thread and I'm just working on a fix

@florianbepunkt
Copy link
Author

@chrisbianca alright. just fyi: I tested above only on ios. if I can help in any way let me know.

@chrisbianca
Copy link
Contributor

@florianbepunkt I've just pushed a fix which I've tested on iOS. Would be great if you're able to confirm that it's working for you now too

@florianbepunkt
Copy link
Author

florianbepunkt commented Dec 6, 2016

@chrisbianca works on ios. tested orderByChild with startAt, endAt and both combined.

@chrisbianca
Copy link
Contributor

Excellent! @Salakar when you're happy, I think this can be closed again

@Ehesp
Copy link
Contributor

Ehesp commented Dec 6, 2016

Also works on my end. Nice one 👍 (Android)

@Salakar
Copy link
Collaborator

Salakar commented Dec 7, 2016

Closing issue as it's now resolved completely 👍

@esdrasportillo
Copy link

There is something bizarre happening here, this same error is happening only in the iPhone 5, both in the simulator and on a real device as well, I am on Salakar's branch, has anyone ever tested on an iPhone 5 before and got this error?

@AndrewHenderson
Copy link

AndrewHenderson commented Mar 25, 2017

I am still not getting the expected results.

firestack.database
      .ref('events')
      .orderByChild('timestamp')
      .endAt(1490331248253)
      .once('value')
      .then((snapshot) => {
        console.log('queryTest orderByChild endAt test > snapshot', snapshot);
      })

This returns all results rather than stopping at the timestamp provided.

The following provides no results at all despite having records in the database that have those exact timestamps:

firestack
      .database
      .ref('events')
      .orderByChild('timestamp')
      .startAt(1490331248253 )
      .endAt(1490331422094)
      .once('value')
      .then((snapshot) => {
        console.log('queryTest orderByChild startAt and endAt test > snapshot', snapshot);
      })

@chrisbianca What push are you referring to? Can you link to the commit?

ping: @Salakar

@Salakar
Copy link
Collaborator

Salakar commented Mar 27, 2017

@AndrewHenderson the above mentioned fixes happened on the v3 version of firestack, your code above looks like you're currently using the old firestack version.

e.g. database became database() on the new version to match the web sdk.

v3 was my fork: https://github.com/Salakar/react-native-firestack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants