Skip to content
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

Would be nice to have session.fetch(...).asPromise() #7

Open
cypherix93 opened this issue Jan 8, 2017 · 13 comments
Open

Would be nice to have session.fetch(...).asPromise() #7

cypherix93 opened this issue Jan 8, 2017 · 13 comments

Comments

@cypherix93
Copy link

cypherix93 commented Jan 8, 2017

Hey there, I've been using this library for a bit now and I love the way every query can be promisified so I can use await on them. However, the fetch function on Session does not have an asPromise function. I would like to have something like the following:

var stuff = new Stuff();
session.save(stuff);
session.flush();

// I would like to have the new Stuff document from the DB at this point.
var dbStuff = await session.fetch(stuff).asPromise();

return dbStuff; // as JSON to front end

Is it possible to do something like this?

Thanks!

@meirgottlieb
Copy link
Contributor

Yeah, that should be OK. I'll work it into the next release.

@meirgottlieb
Copy link
Contributor

meirgottlieb commented Jan 9, 2017

Sorry, I was too quick to respond. Because of the way operations on the session are queued up this is actually a bigger change than I thought. I'm going to have to think about this one. I'll leave the issue open. In the meantime, you can chain fetch with a query and get a promise. For example:

var fetchedStuff = await session.find(Stuff, id).fetch("someReference").asPromise();

@cypherix93
Copy link
Author

Thanks for the prompt responses. The issue I am facing is getting a hold of the _id after a session.save(...) call. I tried looking at the same object reference that was saved, but the _id or id field wasn't populated. That's why I needed to do a fetch. Any tips on how to accomplish that?

I can't use session.find(Stuff, id).fetch(...) because I don't have the id.

@meirgottlieb
Copy link
Contributor

The call to save is async. Although the callback is not required, the id is assigned during the save operation. If you put your code in the callback to save, the id will be available.

@cypherix93
Copy link
Author

Would it then be desirable to have an asPromise() for save as well? Or maybe if the second argument is not passed, it returns a Promise by default? I'll try to put together a PR if I can manage the time.

@meirgottlieb
Copy link
Contributor

Yeah, whatever I do for fetch I'll do for the rest of the methods on Session as well. What I'm leaning toward is returning a Promise whenever a callback is not provided.

@cypherix93
Copy link
Author

That sounds great! Thanks.

@meirgottlieb
Copy link
Contributor

meirgottlieb commented Jan 10, 2017

There is a problem with returning a Promise when the callback is not provided.

Some background: Currently, the callback is already optional on some operations. This is because all operations on the Session are queued. The queue is then processed based on the type of operation. For example multiple find operations are executed in parallel. However, if you call flush, the queue will wait for the flush to complete before processing additional operations.

If a callback is not passed to an operation, and an error occurs, the error is passed to the next call on the Session with a callback. If none are queued then the error is raised as an event on the Session. In order to return a promise when a callback is not provided, a Promise-wrapped callback would have to be passed to the queue. This would break the current error handling.

I'll continue to look into this.

@cypherix93
Copy link
Author

cypherix93 commented Jan 10, 2017

Maybe a solution would be to update the tracked references after flush is called? I don't know how exactly the tracking is implemented in this library, but coming from a C# background, I have noticed that Entity Framework (ORM for .NET) injects the database-generated fields into the tracked model upon calling SaveChanges().

I think an elegant solution would be to keep the current implementation that you have, and implement something similar to what Entity Framework does.

Example for clarity:

var stuff = new Stuff();
//... fill in Stuff fields

session.save(stuff); // goes into the queue for later execution

session.flush(); // executes the save

// at this point the original reference to stuff should have the _id field
console.log(stuff._id) // => "587524fdedab1f0014ff0cb2"

What do you think?

@meirgottlieb
Copy link
Contributor

That's pretty much how it works now. The issue is each of those steps is async and the current way the Session handles errors doesn't exactly mesh with returning Promises, but I have a few ideas. I'll update this thread when I have more.

@edcarroll
Copy link

Sorry to press you, I'm very keen to be able to use this functionality - have you had any further progress with it? 😄

@meirgottlieb
Copy link
Contributor

Doing this will require a breaking change to the API. I haven't put it in place yet.

@edcarroll
Copy link

No problem - thank you for the quick reply, I look forward to the update :)

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

3 participants