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

Interception occurs before PrepareJSONSchema #103

Open
twelvelabs opened this issue Dec 28, 2023 · 2 comments
Open

Interception occurs before PrepareJSONSchema #103

twelvelabs opened this issue Dec 28, 2023 · 2 comments

Comments

@twelvelabs
Copy link

Describe the bug

I would like my schema to include markdownDescription attributes so that VS Code (via the JSON and YAML language servers) will render help text properly in hover overlays. Since I don't want to have to maintain duplicate versions of each description, I was hoping to use InterceptSchema to populate the markdownDescription value automatically.

Unfortunately, many of my descriptions are set via PrepareJSONSchema() (they're long and not conducive to being managed in struct tags). It appears that both the property and schema interceptors are called before PrepareJSONSchema.

To Reproduce

type MySchema struct {
	Something string `yaml:"something"`
}

func (m *MySchema) PrepareJSONSchema(schema *jsonschema.Schema) error {
	schema.Properties["something"].TypeObject.WithDescription(`
		Super long description...
	`)
}

schema, err := (&jsonschema.Reflector{}).Reflect(
	&MySchema{},
	jsonschema.InterceptSchema(func(params jsonschema.InterceptSchemaParams) (bool, error) {
		if params.Processed {
			params.Schema.WithExtraPropertiesItem(
				"markdownDescription", *params.Schema.Description,
			)
		}
		return false, nil
	}),
)

Expected behavior

I expected schema to contain the markdownDescription prop set to the content of the description... sadly, it did not. I had to post-process schema with the following. Which honestly, wasn't that big of a deal. It was just confusing because it seemed like the interceptor approach was the right way to go about it.

func setMarkdownDescription(schema *jsonschema.Schema) {
	if schema.Description != nil {
		schema.WithExtraPropertiesItem(
			"markdownDescription", *schema.Description,
		)
	}
	for _, s := range schema.Properties {
		if s.TypeObject != nil {
			setMarkdownDescription(s.TypeObject)
		}
	}
	for _, s := range schema.Definitions {
		if s.TypeObject != nil {
			setMarkdownDescription(s.TypeObject)
		}
	}
}

Additional context

I don't know if this is technically a bug, or if the behavior is by design. If it's the latter, I think it would be good to document in both interceptor functions to prevent further confusion.

@vearutop
Copy link
Member

Thank you for raising this, I think this should be considered a bug.

Preparer is an attribute of schema (can be used in multiple reflections), while interceptor is an attribute of a reflection. I think it makes sense to trigger preparer before triggering processed interceptor.

However, there is another issue with your example. The problem is that you prepare parent schema, but interceptor is called for schemas as they are built from deeper nodes to outer.

  • start
  • walk properties
    • intercept schema of Something string (no description here yet, but schema is finished and set to parent)
  • prepare schema of MySchema (set property description)
  • intercept schema of MySchema

Let me think how such a (racy?) case can be solved conveniently.

@vearutop
Copy link
Member

Maybe, what could be a solution here is a way to mimic field tags without field tags, so that lengthy or dynamic values can be used.

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

No branches or pull requests

2 participants