edgedb-js icon indicating copy to clipboard operation
edgedb-js copied to clipboard

Object form of `filter(_single)` doesn't reject arbitrary nested objects

Open gomain opened this issue 2 years ago • 3 comments

Code The code causing the error: get Service by id constrained by of (User)

/*
 * typechecks but throws at runtime
 * Error: Invalid value for type default::User: {"id":"5d4fe742-a85f-11ed-931e-c7a8a8ffec99"}
 */
e.select(e.Service, () => ({
  filter_single: {
    id,
    of: { id: userId },
  },
}));

// this works
e.select(e.Service, () => ({
  filter_single: {
    id,
    of: e.select(e.User, () => ({ filter_single: { id: userId } })),
  },
}));

// this works
e.select(e.Service, (service) => ({
  filter_single: e.op(
    e.op(service.id, "=", e.uuid(id)),
    "and",
    e.op(service.of.id, "=", e.uuid(userId)),
  ),
}));

Schema

Your application schema.

One-to-many relation between User and Service with backlink.

module default {

  type User {
    multi link services := .<of[is Service];
  };

  type Service {
    required link of -> User;
  };

};

Generated EdgeQL

Execution of select throws.

Error

personal information redacted

    Error: Invalid value for type default::User: {"id":"cb4e29a4-a862-11ed-b29e-ab480f69db2e"} 
        at literalToEdgeQL (file:///..../edgeql-js/toEdgeQL.mjs:1076:19)
        at renderEdgeQL (file:///..../edgeql-js/toEdgeQL.mjs:568:16)
        at renderEdgeQL (file:///..../edgeql-js/toEdgeQL.mjs:768:70)
        at renderEdgeQL (file:///..../edgeql-js/toEdgeQL.mjs:768:70) 
        at renderEdgeQL (file:///..../edgeql-js/toEdgeQL.mjs:642:38)
        at Proxy.$toEdgeQL (file:///..../edgeql-js/toEdgeQL.mjs:141:18)
        at Object.getService (file:///....)
        at file:///....
        at file:///....
        at processTicksAndRejections (node:internal/process/task_queues:96:5)

Desired behavior

I really wish the first version would have worked. If not, it should not typecheck.

What is the EdgeDB/QL idiomatic way of writing this use case?

Versions (please complete the following information):

  • OS: linux-nixos (local binany)
  • EdgeDB version: 2.9+b1d697b
  • EdgeDB CLI version: 2.2.6+b869667
  • edgedb-js version: 1.0.2
  • @edgedb/generate version: 0.0.7
  • Typescript version: 4.9.5
  • Node version: v16.18.1

gomain avatar Feb 09 '23 10:02 gomain

I really wish the first version would have worked.

Yeah, I agree that we should do more to protect you as the consumer from this at the type-level.

What is the EdgeDB/QL idiomatic way of writing this use case?

For this kind of "owner" <> "owned" relationship, the idiomatic way of structuring this is using globals and access policies.

scotttrinh avatar May 03 '23 16:05 scotttrinh

@scotttrinh Thanks for responding.

I'm a bit wary of using access policies unless it is indeed a user's access policy. Even for that having global mutable state is pushing me over the edge.

My question is rather in regards to general one <> many relationships where we want to query for a many given a one.

gomain avatar May 06 '23 10:05 gomain

@gomain

My question is rather in regards to general one <> many relationships where we want to query for a many given a one.

Gotcha. I think this form is the one that I've seen the most:

// this works
e.select(e.Service, (service) => ({
  filter_single: e.op(
    e.op(service.id, "=", e.uuid(id)),
    "and",
    e.op(service.of.id, "=", e.uuid(userId)),
  ),
}));

and I absolutely get that nesting e.op's is a a bit of a readability and DX pain. Your first version (where you have nested objects) is something that trips up a lot of people since they expect that compact object format to be recursive, but it isn't and as you observed, the compiler doesn't help block you from seeing the mistake there. Definitely going to improve the typing here and also look at doing these and style nested queries easier.

scotttrinh avatar May 08 '23 15:05 scotttrinh