santhosh-tekuri / jsonschema

JSONSchema (draft 2020-12, draft 2019-09, draft-7, draft-6, draft-4) Validation using Go
Apache License 2.0
957 stars 98 forks source link

implementing json.Marshaler interface #154

Closed Cih2001 closed 8 months ago

Cih2001 commented 8 months ago

Hey, will you consider implementing json.Marshaller interface on the Schema? We are currently using https://github.com/qri-io/jsonschema in one project, and we rely on the fact that loaded schemas there can be marshalled back to a byte stream / string. https://github.com/qri-io/jsonschema/blob/780655b2ba0ed03f11ecc8606a8f6e7380bb57be/schema.go#L342

We use this to store loaded schemas in the memory to store in a database a json byte array. We would love to switch to your library, but unfortunately, because of this missing feature, we are unable to do so. Will you consider adding this to the package? I might even be able to help on this one.

Thanks.

santhosh-tekuri commented 8 months ago

this library uses compilation unlike unmarshalling which is done by the library mentioned. So it is not trivial to implement marshalling.

few drawbacks if we implement marshalling feature:

then mentioned library does reference resolution during validation time. see here so it has to update keyword struct during validation. because of this its validation is not thread safe. in fact they have an issue for this.

to conclude, it is possible to implement json.Marshaler. but generated schema may not be identical to original schema:

currently I have no plans to implement this. since Schema struct is public you can implement this outside the library

Cih2001 commented 8 months ago

Thanks Santhosh, we are trying to move away from qri-iolibrary exactly because of the concurrency issue it has. I ended up doing the same as you suggested, creating an internal package that wraps the jsonschema package, where I can implement my own methods.

Thanks for your time and explanation.

rubenhazelaar commented 8 months ago

Thanks Santhosh, we are trying to move away from qri-iolibrary exactly because of the concurrency issue it has. I ended up doing the same as you suggested, creating an internal package that wraps the jsonschema package, where I can implement my own methods.

Thanks for your time and explanation

@Cih2001 Would it be possible for you to share this implementation? I am running into the same issue where I would like to show the schema as JSON through an endpoint.

Cih2001 commented 8 months ago

Unfortunately, I can't share codes with you, as it is an internal implementation. But the idea could be as simple as wrapping the jsonschema.Schema into another structure and store it's raw format alongside.

type Schema struct {
    schema jsonschema.Schema
    raw []byte
}

now you can define your custom marshaller and unmarshaller methods there:

func (s *Schema) UnmarshalJSON(data []byte) error {
    schema, err := jsonschema.CompileString("", string(data))
    if err != nil {
        return err
    }
    s.raw = data
    s.schema = schema
    return nil
}

Similarly, you can define all other methods you need. If you depend heavily on functions provided by the jsonschema pkg, you may consider exporting schema in your custom Schema type.

rubenhazelaar commented 8 months ago

Thank you @Cih2001, our requirements are quite specific as the marshalled schema must be valid and executable by other json schema libraries. This results in merging all refs in $defs of the marshalled schema.

I have implemented it now, quite robustly I believe (although I am not an export on JSON Schema), but I am unsure if this is what you or others are looking for.

@santhosh-tekuri has already indicated (https://github.com/santhosh-tekuri/jsonschema/issues/154#issuecomment-2005757922) he has no plans to implement something like this, which I can understand.

@santhosh-tekuri would you consider looking at a PR in which I integrate my current solution into this library? I do need to make some time for this and of course I respect it if you think this is out-of-scope.

santhosh-tekuri commented 8 months ago

@rubenhazelaar

I recently ported this library to rust (https://github.com/santhosh-tekuri/boon)

while doing this I noticed that current implementation can be improved for better maintenance and some difficult to solve issues can be resolved. So planning to do major rewrite soon.

So not currently in position to accept any new features. I might considering adding marshalling feature but only in new revised implementation

rubenhazelaar commented 8 months ago

Ok, thank you. Very curious about the rewrite and what the improvements will be. For now I will continue to use the current version and our own implementation. Do have some indication about a timeframe?

santhosh-tekuri commented 7 months ago

The rewrite is mainly for better maintenance.

I started it few days back. while reading the boon code, I found a bug in it. currently fighting with rust borrower to fix the bug.

from user point of view there should not be much difference other than minor api changes.

rubenhazelaar commented 7 months ago

Great, thank you. Good luck with your battle against the borrower :)