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

Database Sync impl missing inside Juniper schema #1157

Closed
j-maas opened this issue Oct 14, 2019 · 5 comments
Closed

Database Sync impl missing inside Juniper schema #1157

j-maas opened this issue Oct 14, 2019 · 5 comments
Labels
question A question (converts to discussion) upstream An unresolvable issue: an upstream dependency bug

Comments

@j-maas
Copy link

j-maas commented Oct 14, 2019

Using rocket = "0.4.2". Bug reproduced on Windows 10 and Elementary OS (Ubuntu-based). I've tested on multiple nightly versions.

I'm encountering a bug that appears very similar to #578. Unfortunately, I don't understand what to do about it from their last comment.

I'm trying migrate a ReST API to GraphQL, for which I employ juniper. It's backed by a SQLite database accessed through diesel, for which I use rocket's integration. However, the code refuses to compile, because the Schema type that I need to pass to juniper is not deemed thread-safe:

error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
  --> src/api.rs:27:1
   |
27 | / fn post_graphql_handler(
28 | |     context: Database,
29 | |     request: juniper_rocket::GraphQLRequest,
30 | |     schema: State<Schema>,
31 | | ) -> juniper_rocket::GraphQLResponse {
32 | |     request.execute(&schema, &context)
33 | | }
   | |_^ `std::cell::Cell<i32>` cannot be shared between threads safely
   |
   = help: within `juniper::schema::model::RootNode<'static, store::Query, juniper::types::scalars::EmptyMutation<store::Context>>`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<i32>`
   = note: required because it appears within the type `diesel::connection::AnsiTransactionManager`
   = note: required because it appears within the type `diesel::SqliteConnection`
   = note: required because it appears within the type `rocket_contrib::databases::r2d2::Conn<diesel::SqliteConnection>`
   = note: required because it appears within the type `std::option::Option<rocket_contrib::databases::r2d2::Conn<diesel::SqliteConnection>>`
   = note: required because it appears within the type `diesel::r2d2::PooledConnection<diesel::r2d2::ConnectionManager<diesel::SqliteConnection>>`
   = note: required because it appears within the type `store::db::Database`
   = note: required because it appears within the type `store::Context`
   = note: required because it appears within the type `std::marker::PhantomData<store::Context>`
   = note: required because it appears within the type `juniper::types::scalars::EmptyMutation<store::Context>`
   = note: required because it appears within the type `juniper::schema::model::RootNode<'static, store::Query, juniper::types::scalars::EmptyMutation<store::Context>>`
   = note: required by `rocket::State`

This is one of 9 errors, all being about a missing Sync impl that can be traced back to rocket::State's requirements. The full code is available at https://github.com/Y0hy0h/lindyhop-aachen/tree/bcbc0f02b62e10db2cdf73ae4deb194fe7f6c19a.

Curiously, I can run https://github.com/ELD/rocket-juniper-example just fine after copying the dependencies from my code into their Cargo.toml. But I can't figure out what I did differently than them. I use the Database struct instead of the SqliteConnection in the Context just as them, and I request the Database and State<Schema> as they do.

I hope this is the right place to open this issue. Maybe you can point me to where the problem may lie.

@jebrosen
Copy link
Collaborator

You might also look at https://github.com/davidpdrsn/graphql-app-example as an example of juniper in rocket.

Unfortunately I haven't yet put the time in to try working with juniper, and there's enough generated code involved that I can't seem to figure anything out from looking at the docs, either.

@jebrosen jebrosen added question A question (converts to discussion) upstream An unresolvable issue: an upstream dependency bug labels Oct 14, 2019
@j-maas
Copy link
Author

j-maas commented Oct 15, 2019

@jebrosen Thanks for the example, that's a very different approach. I haven't tried running it, because I the example I linked above is more similar to my code and already runs. The biggest difference in your example is that the GraphQL Schema type is auto-generated from a GraphQL file, instead of being explicitely constructed in Rust. That's, however, the type that seems to cause the problems, so I couldn't pull much more new information out of looking at that example.

@jebrosen
Copy link
Collaborator

Sorry that example didn't help after all.

One thing you seem to be doing differently from the example you posted has to do with the MutationRoot. @ELD, can you explain this difference and why it matters? It feels like I have to learn GraphQL in order to understand how these types actually work.

@ELD
Copy link
Member

ELD commented Oct 15, 2019

@y0hy0h It's been awhile since I've experimented with GraphQL via Juniper. I should probably update my example now that they've expanded their use of procedural macros. I don't have time to look into it right now as I'm at work, but I'll try to provide more support this evening.

My initial guess is that their procedural macro doesn't take something into account that my example isn't affected by since I constructed the Schema type myself. This is all from memory, though, so I don't know if that's correct. I'll try to dig into it a little more, though.

j-maas pushed a commit to j-maas/lindyhop-aachen-rust that referenced this issue Oct 16, 2019
The type was not detected as thread-safe, which made Rocket abort compilation. See rwf2/Rocket#1157.
@j-maas
Copy link
Author

j-maas commented Oct 16, 2019

@jebrosen The Mutation type seems to be it. If instead of using the juniper::EmptyMutation I simply do

pub struct Mutation;

#[juniper::object(Context=Store)]
impl Mutation {}

then it compiles.

So this definitely looks like an issue with juniper to me. I will push this issue upstream.

Thanks a lot for all you kind help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question A question (converts to discussion) upstream An unresolvable issue: an upstream dependency bug
Projects
None yet
Development

No branches or pull requests

3 participants