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

Untagged operation format & schema validation #235

Merged
merged 103 commits into from
Aug 19, 2022

Conversation

sandreae
Copy link
Member

@sandreae sandreae commented Aug 12, 2022

Integrate new API for the "monster" p2panda/p2panda#415 including some changes needed for this PR here p2panda/p2panda#434

  • implement all simple API changes implemented here: Untagged operation format, schema validation, new operation and entry API p2panda#415
  • refactor tests which are simple to update
  • remove tests which are hard to update
  • update domain methods to use new validation methods
  • fetch schema from provider before passing it, entry and operation, into publish
  • correct method for parsing EncodedEntry + EncodedOperation in gql scalar traits
  • re-implement storing entry + payloads which arrive via replication
  • re-implement test utilities send_to_store etc...
  • fix existing tests which are failing
  • bring back all deleted tests
  • add tests for schema validation
  • update doc strings for publish
  • update nextEntryArgs to accept a document view id
  • e2e test poetry 🥞

Next up

📋 Checklist

  • Add tests that cover your changes
  • Add this PR to the Unreleased section in CHANGELOG.md
  • Link this PR to any issues it closes
  • New files contain a SPDX license header

@sandreae
Copy link
Member Author

On the off chance it's related, I'm gunna pull the e2e test out of this PR into it's own. 👍

@adzialocha
Copy link
Member

On the off chance it's related, I'm gunna pull the e2e test out of this PR into it's own. +1

Hmn .. none of these errors are related to the e2e test

@sandreae
Copy link
Member Author

sandreae commented Aug 19, 2022

On the off chance it's related, I'm gunna pull the e2e test out of this PR into it's own. +1

Hmn .. none of these errors are related to the e2e test

I know, but it seems like a new variable (running Node directly) which is worth removing 👍 I tried, it didn't seem to help

@sandreae
Copy link
Member Author

Booo, I just got similar errors:


failures:

---- graphql::client::dynamic_query::test::single_query stdout ----
-------------- TEST START --------------
thread 'graphql::client::dynamic_query::test::single_query' panicked at 'assertion failed: `(left == right)`
  left: `Null`,
 right: `Object({Name("byViewId"): Object({Name("fields"): Object({Name("bool"): Boolean(true)})}), Name("byDocumentId"): Object({Name("fields"): Object({Name("bool"): Boolean(true)})})})`: [
    ServerError {
        message: "Unknown field \"schema_name_0020ed39a1975cc0dfb369cb54bcd662ebf29c4563af22cfc8feb578eb5ba58f9b13\" on type \"QueryRoot\".",
        locations: [
            Pos(2:17),
        ],
        path: [],
        extensions: None,
    },
    ServerError {
        message: "Unknown field \"schema_name_0020ed39a1975cc0dfb369cb54bcd662ebf29c4563af22cfc8feb578eb5ba58f9b13\" on type \"QueryRoot\".",
        locations: [
            Pos(5:17),
        ],
        path: [],
        extensions: None,
    },
]', aquadoggo/src/graphql/client/dynamic_query.rs:440:13

---- graphql::client::dynamic_types::tests::application_schema_container_type stdout ----
-------------- TEST START --------------
thread 'graphql::client::dynamic_types::tests::application_schema_container_type' panicked at 'assertion failed: `(left == right)`
  left: `Object({Name("schema"): Null})`,
 right: `Object({Name("schema"): Object({Name("kind"): String("OBJECT"), Name("name"): String("schema_name_0020901c9ab87fa9da3e44ce56a507bcd0ff5075354bb3acdf663bbe540459a11610"), Name("description"): String("test schema description"), Name("fields"): List([Object({Name("name"): String("meta"), Name("type"): Object({Name("name"): String("DocumentMeta")})}), Object({Name("name"): String("fields"), Name("type"): Object({Name("name"): String("schema_name_0020901c9ab87fa9da3e44ce56a507bcd0ff5075354bb3acdf663bbe540459a11610Fields")})})])})})`: 
[]
', aquadoggo/src/graphql/client/dynamic_types/tests.rs:134:9

---- graphql::replication::query::tests::entries_newer_than_seq_num_cursor::case_3_some_edges_no_next_page stdout ----
-------------- TEST START --------------
thread 'graphql::replication::query::tests::entries_newer_than_seq_num_cursor::case_3_some_edges_no_next_page' panicked at 'Send to store: Custom("Entry's claimed log id of 1 does not match existing log id of 0 for given author and document")', /home/sandreae/.cargo/git/checkouts/p2panda-fb44b351afe3c13e/103a133/p2panda-rs/src/test_utils/db/test_db.rs:250:26


failures:
    graphql::client::dynamic_query::test::single_query
    graphql::client::dynamic_types::tests::application_schema_container_type
    graphql::replication::query::tests::entries_newer_than_seq_num_cursor::case_3_some_edges_no_next_page

Which vanished on a second run 😭

@sandreae
Copy link
Member Author

(That was after removing the e2e test)

@sandreae
Copy link
Member Author

When it happens, it seems likely graphql::client::dynamic_query::test::single_query will be in there.

@sandreae
Copy link
Member Author

sandreae commented Aug 19, 2022

Hmm, every test I've seen fail has been using the graphql interface.

@sandreae
Copy link
Member Author

Maybe increasing the broadcast::channel capacity higher that 16 is making a difference....

@sandreae
Copy link
Member Author

Maybe increasing the broadcast::channel capacity higher that 16 is making a difference....

Fingers crossed, I think that was it. 16 seemed to be too low.

Also in the e2e tests, when running the tests with RUST_LOG=trace we need to wait longer after publishing an entry for the node to process it.

@sandreae sandreae marked this pull request as ready for review August 19, 2022 16:19
@@ -0,0 +1,402 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

//! E2E test and mini-tutorial for aquadoggo.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very beautiful! 🌹

I've renamed the file to tests.rs as the place of the file in the root already indicates that it is "end-to-end".

@adzialocha adzialocha self-requested a review August 19, 2022 17:36
Copy link
Member

@adzialocha adzialocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yesssssss, this is the final step and it looks great! おつかれさまでした!

I think all other things can be easily tackled in sub-sequent steps, for now its just important that we have everything in place ✨


Small bummer: I continue having some failing cases when running tests .. much more rare now though. I propose fixing this in another PR though as this is not the scope of this one. I changed the target branch to development for now 👍

---- graphql::client::dynamic_types::tests::application_schema_fields_type stdout ----
-------------- TEST START --------------
thread 'graphql::client::dynamic_types::tests::application_schema_fields_type' panicked at 'assertion failed: `(left == right)`
  left: `Object({Name("schemaFields"): Null})`,
 right: `Object({Name("schemaFields"): Object({Name("description"): String("Data fields available on documents of this schema."), Name("fields"): List([Object({Name("name"): String("bool_field"), Name("type"): Object({Name("kind"): String("SCALAR"), Name("name"): String("Boolean")})}), Object({Name("name"): String("list_field"), Name("type"): Object({Name("kind"): String("LIST"), Name("name"): Null})}), Object({Name("name"): String("relation_field"), Name("type"): Object({Name("kind"): String("OBJECT"), Name("name"): String("schema_definition_v1")})})])})})`:
[]
', aquadoggo/src/graphql/client/dynamic_types/tests.rs:216:9

.. and the good old:

thread 'graphql::client::dynamic_query::test::single_query' panicked at 'assertion failed: `(left == right)`
  left: `Null`,
 right: `Object({Name("byViewId"): Object({Name("fields"): Object({Name("bool"): Boolean(true)})}), Name("byDocumentId"): Object({Name("fields"): Object({Name("bool"): Boolean(true)})})})`: [
    ServerError {
        message: "Unknown field \"schema_name_0020b26f0944a65929c85f9008dfe0de57b3a6b992e398b421a03d5f405964b89617\" on type \"QueryRoot\".",
        locations: [
            Pos(2:17),
        ],
        path: [],
        extensions: None,
    },
    ServerError {
        message: "Unknown field \"schema_name_0020b26f0944a65929c85f9008dfe0de57b3a6b992e398b421a03d5f405964b89617\" on type \"QueryRoot\".",
        locations: [
            Pos(5:17),
        ],
        path: [],
        extensions: None,
    },
]', aquadoggo/src/graphql/client/dynamic_query.rs:440:13

@adzialocha adzialocha changed the base branch from main to development August 19, 2022 17:48
@sandreae
Copy link
Member Author

👏👏👏👏🥳🥳🥳

@sandreae sandreae merged commit f95cf83 into development Aug 19, 2022
adzialocha added a commit that referenced this pull request Aug 22, 2022
* development:
  clippy
  Use `DocumentStore` trait from p2panda rs (#249)
  Corrections in e2e test
  Untagged operation format & schema validation (#235)
adzialocha added a commit that referenced this pull request Aug 22, 2022
* development:
  Friendly names for scalars in GraphQL schema (#231)
  clippy
  Use `DocumentStore` trait from p2panda rs (#249)
  Corrections in e2e test
  Untagged operation format & schema validation (#235)
adzialocha added a commit that referenced this pull request Aug 22, 2022
* development:
  Fix GraphQL responses (#236)
  Friendly names for scalars in GraphQL schema (#231)
  clippy
  Use `DocumentStore` trait from p2panda rs (#249)
  Corrections in e2e test
  Untagged operation format & schema validation (#235)
adzialocha added a commit that referenced this pull request Aug 22, 2022
* development:
  Rename `nextEntryArgs` and `publishEntry` (#232)
  Fix GraphQL responses (#236)
  Friendly names for scalars in GraphQL schema (#231)
  clippy
  Use `DocumentStore` trait from p2panda rs (#249)
  Corrections in e2e test
  Untagged operation format & schema validation (#235)
@adzialocha adzialocha deleted the the-monster-refactor-again branch August 22, 2022 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants