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

Support external references #621

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

SRetip
Copy link
Contributor

@SRetip SRetip commented Jul 1, 2024

Implementation for issue #201.

These changes were made based on the discussion in the issue.

I understand that this pull request can be improved, and I would appreciate any feedback and thoughts on how to make it better.

Comment on lines 23 to 24
pathdiff = "0.2.1"
iref = "3.1.4"
Copy link
Contributor

Choose a reason for hiding this comment

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

the dependencies above are alpha sorted. would be good for these new ones to also be alpha sorted

let reader = BufReader::new(file);

let schema: RootSchema = serde_json::from_reader(reader).unwrap();
// schema.schema.metadata().title = Some("Everything".to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

commented out code in this test

@SRetip SRetip requested a review from jayvdb August 6, 2024 08:15
@baxterjo
Copy link

@jayvdb Bumping this

@liamwh
Copy link

liamwh commented Nov 19, 2024

@SRetip is it an idea that you resolve the merge conflicts so that the maintainers can approve and this can be merged?

@LimpidCrypto
Copy link

I'm really looking forward to this feature. I already tried using the feature branch in my project:

typify = { git = "https://github.com/agily/typify", branch = "support-external-references", package = "typify" }

I have one file schemas/common/xrp.json:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "https://raw.githubusercontent.com/LimpidCrypto/xrpl-models/main/schemas/common/xrp.json",
    "title": "XRP",
    "type": "object",
    "properties": {
        "currency": {
            "type": "string",
            "const": "XRP",
            "description": "The currency code for XRP."
        },
        "issuer": {
            "$ref": "https://raw.githubusercontent.com/LimpidCrypto/xrpl-models/main/schemas/types.json#/definitions/ACCOUNT",
            "description": "The empty string, because XRP is not issued by any single entity."
        }
    },
    "required": [
        "currency",
        "issuer"
    ]
}

In the properties.issuer I'm trying to reference a type in schemas/types.json:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "https://raw.githubusercontent.com/LimpidCrypto/xrpl-models/main/schemas/types.json",
    "definitions": {
        "ACCOUNT": {
            "type": "string",
            "description": "An XRP Ledger account address.",
            "pattern": "^r[1-9A-HJ-NP-Za-km-z]{25,34}$"
        }
    }
}

In my project I'm trying to import XRP like so:

import_types!(schema="./schemas/common/xrp.json");

But I get this uninformative error message:

error: proc macro panicked
 --> src/common/xrp.rs:4:1
  |
4 | import_types!(schema="./schemas/common/xrp.json");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: message: called `Option::unwrap()` on a `None` value

I'm new to json schemas and this library so it's very likely I just have a false understanding on how to reference to a type externally.

if fragment.is_empty() {
return Schema::Object(base_schema.schema.clone());
}
let definition_schema = if fragment[0] == "definitions" {

Choose a reason for hiding this comment

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

Suggested change
let definition_schema = if fragment[0] == "definitions" {
let definition_schema = if ["definitions", "$defs"].contains(&fragment[0].as_str()) {

This should match both "definitions" and "$defs" as "https://json-schema.org/draft/2020-12/schema" replaced "definitions" with "$defs"

@LimpidCrypto
Copy link

My review above resolves my issue using typify-impl but still exists using typify-macro. It seems like it does not resolve the base_path in typify-impl/lib.rs::fetch_external_definitions. It's just an empty string. This is an issue when trying to determine the file_path here because of the .parent().unwrap().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants