davidpdrsn / juniper-eager-loading

Library for avoiding N+1 query bugs with Juniper
68 stars 8 forks source link

[discussion] Decouple from database #1

Closed LegNeato closed 5 years ago

LegNeato commented 5 years ago

Hi! This is awesome and looks very similar to Facebook's EntLoader ("entity loader") internal framework. I am rebuilding it in Rust and hadn't got to this piece yet...so I am excited about this project!

A couple things I want to throw out there:

  1. Does this pattern need to be database-specific? For example, if you wanted to back the models/entities with calls to a rest API, I think it would fit into this model. I was planning to have a SqlBackedEnt and a RestBackedEnt and maybe a GrpcBackedEnt. The the datastore would be an implementation detail of the higher-level interface. It looks like that might actually be currently the case, minus the naming of stuff like _db_?
  2. Have you seen https://github.com/cksac/dataloader-rs? Dataloader was created at FB and was plugged into this layer, as it is a natural place to coalescence calls. I see you see the same thing as you have a caching layer.

Hope you don't mind me poking my nose in here while you are experimenting.

davidpdrsn commented 5 years ago

You're of course very welcome to poke your nose around. Thanks for the questions!

  1. Being 100% database agnostic is a major goal with this library. Since I use Diesel myself I do plan to have some derives specifically for Diesel, but there will be no limitations on the backing store. Currently all you have to do is implement this trait https://github.com/davidpdrsn/juniper-eager-loading/blob/master/juniper-eager-loading/src/lib.rs#L354 for each type you need to eager load. That defines how a list of ids for some type gets loaded in the best way. The Diesel derive I'm working on does a select * from $table where ids in any(...) style query. So yes the models could totally come from an external web service. All traits have an associated type called Connection, but the name might be deceiving, it doesn't require to be a database connection. It can be whatever you need.

  2. Yes I looked at it some months ago, but am I right that it requires using futures? Is it compatible with Juniper?

This library is still very experimental so I'm very open to learning from the dataloader pattern. I'm not super familiar with it honestly.

My main goal is to make N+1 queries a thing of the past when using Juniper backed by a relational database, but without forcing users to a specific data store or library.

LegNeato commented 5 years ago

So that load trait is very similar to what I have:

/// A privacy-aware entity.
pub trait Ent<V, T, E>
where
    Self: Sized,
{
    fn load(viewer: &V, id: &T) -> BoxFuture<'static, Result<Self, E>>;
}

One of the insights Facebook had was that the permission layer is intimately tied with data fetching and the permission system generally lives in application code. How this manifests is that all entity load/save code has a ViewerContext and can approve or deny saves and loads based on who is doing the action. And of course the underlying data store (like a database or REST api, or in FB's case TAO) just lets everything through as the application layer has checked the authorization.

This is what product code usage would look like:

/// An opaque id for the `Foo` entity.
#[derive(Debug, Clone, PartialEq, From)]
struct FooId(String);
impl EntId<Error> for FooId {
    fn validation_rules() -> Vec<Box<dyn ValidationRule<Error>>> {
        vec![]
    }
}

/// An entity representing `Foo`.
struct EntFoo {
    id: FooId,
}
impl Ent<(), FooId, Error> for EntFoo {
    fn load(_viewer: &(), id: &FooId) -> BoxFuture<'static, Result<Self, Error>> {
        if *id == FooId::from_str("ID_THAT_SHOULD_FAIL").unwrap() {
            Box::pin(future::ready(Err(Error::new(ErrorKind::Other, "oh no!"))))
        } else {
            Box::pin(future::ready(Ok(EntFoo { id: id.clone() })))
        }
    }
}

#[test]
fn it_works() {
    futures::executor::block_on(async {
        // Loading entity succeeds.
        let id = FooId::from_str("FAKE").unwrap();
        let foo = EntFoo::load(&(), &id).await;
        assert_eq!(foo.unwrap().id, id);

        // Loading entity errors.
        let id = FooId::from_str("ID_THAT_SHOULD_FAIL").unwrap();
        let foo = EntFoo::load(&(), &id).await;
        assert!(foo.is_err());
    })
}
davidpdrsn commented 5 years ago

Hm I haven’t given permissions much thought. Maybe I’m misunderstanding something, but what is the reason the individual resolvers can’t handle the permissions themselves?

davidpdrsn commented 5 years ago

@LegNeato Version 0.1.0 is now on crates.io https://crates.io/crates/juniper-eager-loading 🎊

I have spent quite a long time on the docs so hopefully everything makes sense.

davidpdrsn commented 5 years ago

cc @theduke

LegNeato commented 5 years ago

Looks great, excited to play around with it this week!