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

modifying documents in compiler context #420

Closed
lrlna opened this issue Jan 13, 2023 · 17 comments
Closed

modifying documents in compiler context #420

lrlna opened this issue Jan 13, 2023 · 17 comments
Assignees
Labels
apollo-compiler issues/PRs pertaining to semantic analysis & validation
Milestone

Comments

@lrlna
Copy link
Member

lrlna commented Jan 13, 2023

Context

It's becoming more important for the compiler to be able to modify and work on modified versions of its type system. While compiler's internals should remain immutable it'd be useful to come up with an API to return a modified version of a given document in either a String or perhaps a HIR format that could be accessed programmatically.

Use cases

  • creating API Schema (modifying original type system to remove field marked @inaccessible)
  • modifying executable documents to remove certain fields
  • break down executable documents into several documents

Possible example

Accessing values of API Schema:

    let schema = Path::new("crates/apollo-compiler/examples/documents/type_system.graphql");
    let src = fs::read_to_string(schema).expect("Could not read type system file.");
    compiler.add_type_system(&src, schema);
   
    let query = Path::new("crates/apollo-compiler/examples/documents/get_dog_name.graphql");
    let src = fs::read_to_string(query).expect("Could not read executable file.");
    compiler.add_executable(&src, query);
    
    // modified version of the type system that has `@inaccessible` fields removed
    let api_type_system = compiler.db.api_type_system(); 
    // should probably have the exact same surface level api as the original type system?
    let query_obj_type = api_schema.find_object_type_by_name("Query".into()); 

I am not entirely sure how the internals of the compiler need to change to accommodate this just yet (aside from saying let's not mutate the state!). I think it'd be good to gather a few more examples of how we'd like this to be used before we settle on the architecture and design. If anyone has any specific examples of how they'd like to use modifying documents with the compiler, please bring them forward!

@lrlna lrlna added the apollo-compiler issues/PRs pertaining to semantic analysis & validation label Jan 13, 2023
@Geal
Copy link
Contributor

Geal commented Jan 13, 2023

The design of immutable collection libraries could provide some inspiration here. Instead of modifying, it's building a new view that reuses elements of the old one. As.an example, if you have a tree, you can "insert" or "delete" by building a new tree that reuses some sub trees

@lrlna
Copy link
Member Author

lrlna commented Jan 17, 2023

As.an example, if you have a tree, you can "insert" or "delete" by building a new tree that reuses some sub trees

iirc that's how rowan deals with reparsing, which i think should tie together very nicely with this API as well

@lrlna
Copy link
Member Author

lrlna commented Jan 17, 2023

something i also just thought about is that it's likely that examples such as APISchema creation will actually have to live outside the compiler. Which means we should be providing the semantics and architecture to deal with context modification, but also allow users to specify how they want it done.

^ perhaps was already implicit part of this issue, but better be said out loud

@SimonSapin
Copy link
Contributor

Converting to apollo-encoder types, modifying as needed, then converting back sounds ok (even if the latter involves reparsing). However apollo-encoder would need to gain many more API for mutation. For example ObjectDefinition::field allows adding a field, but there is no way that I can find to remove or modify an existing one. Similarly for every type representing a piece of GraphQL document.

@lrlna
Copy link
Member Author

lrlna commented Feb 8, 2023

something to think about here is that this could be quite similar to a macro system/macro rewrite rules.

i'd be keen to gather a few examples of how we could possibly modifying either the type system or executable definitions.

There is currently an example of adding an additional directive @b to all fields that already have directive @a. So this exectuable definition,

query ExampleQuery {
  topProducts {
    name @a
  }
  ... multipleSubscriptions
}

fragment multipleSubscriptions on Subscription {
  newMessage @a {
    body
    sender
  }
}

will become this:

query ExampleQuery {
  topProducts {
    name @a @b
  }
  ... multipleSubscriptions
}

fragment multipleSubscriptions on Subscription {
  newMessage @a @b {
    body
    sender
  }
}

@SimonSapin @Geal do you have other examples of how you'd like to see executable definitions be expanded? I remember there being something about splitting up queries before?

@Geal
Copy link
Contributor

Geal commented Feb 9, 2023

yes, another idea was that when you get

query ExampleQuery {
  topProducts {
    name @a
    reviews {
      body
    }
  }
}

You could get two different queries from there:

query ExampleQuery {
  topProducts {
    reviews {
      body
    }
  }
}
query ExampleQuery {
  topProducts {
    name 
  }
}

That's more or less what is done with @defer right now

@Geal
Copy link
Contributor

Geal commented Feb 9, 2023

in apollographql/router#2489 we introduced query rewrites that change a field name if there can be some incompatibilities due to interfaces (cf apollographql/federation#1257 ) so a subgraph query can be changed to alias a field

@Geal
Copy link
Contributor

Geal commented Feb 9, 2023

could this be considered a particular case of incremental reparsing? Do we get access to rowan's green nodes from the compiler?

@lrlna
Copy link
Member Author

lrlna commented Feb 9, 2023

compiler.db.document() gives you the top level green node, so theoretically you do get access to the green node. it is a bit harder to do something like compiler.db.find_operation("getName").lower(), as in, we don't have a way to do that yet. i imagine coming up with a graceful way of handing document modification will require us to figure out a way to lower certain HIR nodes in order to modify them

@lrlna
Copy link
Member Author

lrlna commented Feb 9, 2023

another example. given this type system:

directive @a(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
directive @b on OBJECT | FIELD_DEFINITION

type Query {
    topProducts: Product
    customer: User
}

type Product {
    type: String
    price(setPrice: Int): Int
    reviews: [Review] @b
}

type Review {
    body: String
    author: User
}

type User @b {
    id: ID
    name: String
}

with original executable looking like this:

query MyQuery {
    topProducts {
        type
        reviews {
            body
        }
    }

    customer {
        name
    }
}

modified executable should look like this:

query after:
query MyQuery($isB: Boolean) {
  topProducts {
    type
    reviews @a(if: $isB) {
      body
    }
  }
  customer @a(if: $isB) {
    name
  }
}

@zackangelo
Copy link
Contributor

The design of immutable collection libraries could provide some inspiration here. Instead of modifying, it's building a new view that reuses elements of the old one. As.an example, if you have a tree, you can "insert" or "delete" by building a new tree that reuses some sub trees

Any data structures that maintain their history upon mutation are known as "Persistent Data Structures" in FP circles.

@zackangelo
Copy link
Contributor

Converting to apollo-encoder types, modifying as needed, then converting back sounds ok (even if the latter involves reparsing). However apollo-encoder would need to gain many more API for mutation. For example ObjectDefinition::field allows adding a field, but there is no way that I can find to remove or modify an existing one. Similarly for every type representing a piece of GraphQL document.

I'm currently round tripping through the compiler in this way to make modifications to HIR types. I've added my own mutation methods to the encoder types, I'd be happy to open a PR if it'd be useful!

@lrlna
Copy link
Member Author

lrlna commented Mar 6, 2023

@zackangelo what sorts of modifications are your currently implementing? is it along the lines of removing/adding fields? or something else entirely?

@zackangelo
Copy link
Contributor

@lrlna currently I'm removing fields and implemented interfaces. I'm also renaming types.

@SimonSapin
Copy link
Contributor

Speaking of renaming, after a first step that’s equivalent removing an item and adding a new one with a different name, it’d be awesome if apollo-rs could provide support for renaming and automatically updating all existing references to that name

@lrlna
Copy link
Member Author

lrlna commented Mar 10, 2023

Speaking of renaming, after a first step that’s equivalent removing an item and adding a new one with a different name, it’d be awesome if apollo-rs could provide support for renaming and automatically updating all existing references to that name

heheheh apollo-rs entering its LSP era

@lrlna lrlna added this to the apollo-compiler@0.9.0 milestone Apr 3, 2023
@lrlna lrlna self-assigned this May 24, 2023
Geal added a commit to apollographql/router that referenced this issue Jun 14, 2023
Fixes #3150
Related: apollographql/apollo-rs#420

This reintroduces the query planner plugins we had in the past, but with
a different API, and for internal use only for now, until we are sure
about the API's shape.

## Plugin API

The plugins need access to a compiler instance to observe and modify the
query, so the caching query planner adds it as part of the query planner
request (which means that now the cache and bridge get different request
types). For now, if a plugin needs to modify the query, it must
serialize the modified version to a string then reparse it, but the goal
is to support modification right inside the query:
apollographql/apollo-rs#420
That compiler will hold both the schema's type info, and the query.

Once the requests reaches the bridge query planner, it will generate:
- a query plan from the filtered query
- selections from the filtered query (the `Query` object used for
response formatting)
- selections from the original query

Thus execution will depend on the filtered query, so if some fields are
added or removed, it wil change the generated query plan. The plan and
selections will then be cached in the same way as before.

## Details on selections

Query planner plugins can modify the query before sending it through
query planning. They might require different sets of fields on the same
entity, and the query plan might add its own list of fields (keys for
federated queries).
As an example, We could have a User entity with its email field used as
key, and want to remove private information from the query.

So we could have the query:

```graphql
{
  topProducts {
    name
    reviews {
      author {
        email
        username
      }
    }
  }
}
```

So here we would filter the query as follows:

```graphql
{
  topProducts {
    name
    reviews {
      author {
        username
      }
    }
  }
}
```

But since the email is used as key, it would appear in the JSON object
that accumulates response data.
To avoid sending non requested data to the client, we have the response
formatting phase, that uses selections extracted from the query, to pick
only what is needed from the JSON object, and send it to the client.

Here we need to apply the selections of the filtered query, to make sure
the email is not returned to the client (the original query would let it
go through). But we also need to apply the selections from the original
query, to have the null propagation algorithm apply and return a
response in the shape that matches the client query.

There is probably a better way to do it than applying selections twice,
but here we are sure the behaviour will be correct, and so far the
formatting phase is fast enough, we can spend a bit more time there

Co-authored-by: Simon Sapin <simon@apollographql.com>
Co-authored-by: Jeremy Lempereur <jeremy.lempereur@iomentum.com>
Co-authored-by: Gary Pennington <gary@apollographql.com>
@SimonSapin
Copy link
Contributor

I think we can consider this solved with the release of apollo-compiler 1.0 beta: https://docs.rs/apollo-compiler/1.0.0-beta.1/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apollo-compiler issues/PRs pertaining to semantic analysis & validation
Projects
None yet
Development

No branches or pull requests

5 participants