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

$ref doesn't work when ref'ed message also has extends #408

Closed
jmazzitelli opened this issue Aug 27, 2015 · 8 comments
Closed

$ref doesn't work when ref'ed message also has extends #408

jmazzitelli opened this issue Aug 27, 2015 · 8 comments
Milestone

Comments

@jmazzitelli
Copy link

I have one schema that extends another using $ref like this:

SimpleMessage.schema.json:

{
  "type": "object",
  "extends": {
    "$ref": "AuthMessage.schema.json"
  },
  "javaType": "org.abc.SimpleMessage",
  "additionalProperties": false,
  "properties": {
    "msg": {
      "type": "string"
    }
  },
  "required": ["msg"]
}

AuthMessage.schema.json:

{
  "type": "object",
  "extends": {
    "type": "object",
    "javaType": "org.abc.BasicMessage"
  },
  "javaType": "org.abc.AuthMessage",
  "additionalProperties": false,
  "properties": {
    "username": {
      "type": "string"
    },
    "password": {
      "type": "string"
    }
  }
}

As you see in the AuthMessage schema, I have my own org.abc.BasicMessage class that I want my AuthMessage to extend. I am expecting the generated java class declarations to look like this:

public class SimpleMessage extends AuthMessage { ... }
public class AuthMessage extends BasicMessage { ... }

However, what really happens is they both extend BasicMessage:

public class SimpleMessage extends BasicMessage { ... }
public class AuthMessage extends BasicMessage { ... }

It is quite possible I misunderstand how $ref is to work inside extends, but on its face, this looks like a bug. I have $ref referring to AuthMessage in my SimpleMessage schema, but SimpleMessage doesn't extend AuthMessage - it extends BasicMessage (which only AuthMessage should - SimpleMessage should extend AuthMessage). As it is generated now, SimpleMessage doesn't get any properties from AuthMessage (that is, SimpleMessage doesn't get setUsername() and setPassword() since it doesn't extend AuthMessage).

Hopefully, that made sense.

@joelittlejohn
Copy link
Owner

I don't seem to be able to replicate this at all, when I try the schemas you have provided, I get:

public class SimpleMessage
    extends AuthMessage

public class AuthMessage
    extends BasicMessage

Is it possible your project hasn't been cleaned and that the generated types are out-of-date and don't match your schema?

@joelittlejohn
Copy link
Owner

Is it possible that BasicMessage is marked final?

@jmazzitelli
Copy link
Author

BasicMessage is abstract.

public abstract class BasicMessage { ... }

Let me try to put together a simple test project so you can replicate. I'll get back to you.

@jmazzitelli
Copy link
Author

===UPDATE: SKIP TO THE NEXT COMMENT FOR A SMALLER REPLICATION PROJECT TO USE===
I will try to shrink this down to a minimal project [note: I did, see next comment], but clone this: https://github.com/jmazzitelli/hawkular-bus and build it (without testing to make it go faster): "mvn clean install -DskipTests" and then look at the generated source at "hawkular-bus/hawkular-bus-samples/hawkular-bus-sample-jsonschema/target/java-gen/org/hawkular/bus/sample/msg"

FWIW: here is what BasicMessage looks like:

https://github.com/jmazzitelli/hawkular-bus/blob/master/hawkular-bus-common/src/main/java/org/hawkular/bus/common/BasicMessage.java

@jmazzitelli
Copy link
Author

Ok, much smaller replication project.

git clone git@github.com:jmazzitelli/test.git
cd test/jsonschema-issue408
mvn clean install

Look in "target/java-gen/org/abc" and see that AuthMessage.java and SimpleMessage.java both define classes that extend BasicMessage. Here are the schemas used:

https://github.com/jmazzitelli/test/tree/master/jsonschema-issue408/src/main/resources/schema

@jmazzitelli
Copy link
Author

Here's another interesting thing - in my small project, comment out the dependency on the hawkular library (the one that pulls in BasicMessage) and run "mvn clean install" again. BasicMessage.java gets generated for you, but yet still both SimpleMessage and AuthMessage extend BasicMessage.

@jmazzitelli
Copy link
Author

I think I see the problem, but don't know how to fix.

See org.jsonschema2pojo.rules.ObjectRule.getSuperType():

        if (node.has("extends")) {
            superType = ruleFactory.getSchemaRule().apply(nodeName + "Parent", node.get("extends"), jClassContainer, schema);
        }

This in turn calls org.jsonschema2pojo.rules.SchemaRule.apply():

        if (schemaNode.has("$ref")) {
            schema = ruleFactory.getSchemaStore().create(schema, schemaNode.get("$ref").asText());
            schemaNode = schema.getContent();

            if (schema.isGenerated()) {
                return schema.getJavaType(); // <---------- HERE!!!
            }

As I step through the code, I see this is parsing the SimpleMessage schema, and when it gets here the $ref is referring to AuthMessage.schema.json. SchemaRule will look it up, store AuthMessage schema in the "schema" variable at which point in time schema.getJavaType() returns "BasicMessage" and that is returned as the super type of SimpleMessage. Which is not right. It should be returning "AuthMessage" as the super type.

I am stepping through the source code from version 0.4.14

@joelittlejohn
Copy link
Owner

Thanks for the detailed report here, it really helped track down this subtle issue. It's dependent on the order in which schemas are read so was difficult to replicate.

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