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

if we have same struct name, then we will got the same $ref #42

Open
haoel opened this issue Sep 7, 2022 · 3 comments
Open

if we have same struct name, then we will got the same $ref #42

haoel opened this issue Sep 7, 2022 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@haoel
Copy link

haoel commented Sep 7, 2022

Assuming we have two different packages HTTP and TCP, and they all have a Config structure.

package http

type Config struct {
	URL    string `json:"url" jsonschema:"required,format=uri"`
	Method string `json:"method" jsonschema:"required"`
}
package tcp

type Config struct {
	Host string `json:"host" jsonschema:"required,format=hostname"`
	Port int    `json:"port" jsonschema:"required,minimum=1,maximum=65535"`
}

then, we have a struct involving these two packages

package server

type AllConfig struct {
	TCP  tcp.Config  `json:"tcp" jsonschema:"omitempty"`
	HTTP http.Config `json:"http" jsonschema:"omitempty"`
}

then we will have the same $ref = #/$defs/Config for http and tcp in JSON Schema, but actually, the http.Config is quite different with tcp.Config, this causes the incorrect the JSON schema file.

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "https://github.com/invopop/jsonschema/test/all-config",
  "$ref": "#/$defs/AllConfig",
  "$defs": {
    "AllConfig": {
      "properties": {
        "tcp": {
          "$ref": "#/$defs/Config"
        },
        "http": {
          "$ref": "#/$defs/Config"
        }
      },
      "additionalProperties": false,
      "type": "object",
      "required": ["tcp", "http"]
    },
    "Config": {
      "properties": {
        "host": {
          "type": "string",
          "format": "hostname"
        },
        "port": {
          "type": "integer",
          "maximum": 65535,
          "minimum": 1
        }
      },
      "additionalProperties": false,
      "type": "object",
      "required": ["host", "port"]
    }
  }
}

So, is there a way we can define the $ref name to make sure they are using different $defs.

@metafates
Copy link

metafates commented Oct 30, 2022

As a workaround, you can use this

r := new(jsonschema.Reflector)
r.Namer = func(t reflect.Type) string {
    name := t.Name()
    switch name {
    case "Config", /* any other conflicting structs */ "...":
        return filepath.Base(t.PkgPath()) + "." + name
    }

    return name
}

@haoel
Copy link
Author

haoel commented Oct 31, 2022

The example just shows how to reproduce this bug. The workaround here is not a workaround because it solves this issue case by case.

I think we can add the package name as the prefix of the ref. And this would be a better workaround. BTW, I've already done it, refer to https://github.com/megaease/easeprobe/blob/8a29940850fe335fe91fd085835c039bd6c745a1/conf/conf.go#L154-L170

And I hope this issue would be addressed officially.

@samlown
Copy link
Contributor

samlown commented Sep 6, 2023

I'm honestly not sure there is a good solution here with the current implementation. It'd require support for namespaces, and determining those namespaces from package names could be complex (it's not something I've looked at, so not entirely sure.)

I'll leave this one open in case anyone would like to propose a solution in a PR. I good approach could be to add a AddPackageNamespaces boolean parameter that will attempt to add the package name in IDs.

@samlown samlown added enhancement New feature or request help wanted Extra attention is needed labels Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants