jsonschema2pojo icon indicating copy to clipboard operation
jsonschema2pojo copied to clipboard

Please create objects in the definition section to be used

Open AceHack opened this issue 5 years ago • 12 comments
trafficstars

I would have thought the following would work:

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "type": "object",
  "additionalProperties": false,
  "definitions": {
    "MessageAttribute": {
      "type": "object",
      "additionalProperties": true,
      "properties": {
        "Type": {
          "type": "string"
        },
        "Value": {
          "type": "string"
        }
      }
    }
  },
  "properties": {
    "MessageAttributes": {
      "existingJavaType": "java.util.Map<String, MessageAttribute>",
      "type": "object",
      "additionalProperties": { "$ref": "#/definitions/MessageAttribute" }
    }
  }
}

But the MessageAttribute class is not created so the existingJavaType fails. I can work around this problem today in an ugly way but it would be nice for the above syntax to work. The ugly way is to move everything in the definitions section to another file as below:

message-attribute.json

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "type": "object",
  "additionalProperties": true,
  "properties": {
    "Type": {
      "type": "string"
    },
    "Value": {
      "type": "string"
    }
  }
}

message.json

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "type": "object",
  "additionalProperties": false,
  "properties": {
    "MessageAttributes": {
      "existingJavaType": "java.util.Map<String, MessageAttribute>",
      "type": "object",
      "additionalProperties": { "$ref": "message-attribute.yaml" }
    }
  }
}

AceHack avatar Apr 08 '20 16:04 AceHack

it's not really because of MessageAttribute has not been created. This is because existingJavaType can't sometimes resolve MessageAttribute.

The additional problem is classes are moved around more likely chaotically rather than under your guidance, see #1107 for this.

eirnym avatar Apr 09 '20 07:04 eirnym

I suspect you're right and the "definitions" section is not being processed. However, I haven't dug into the code to confirm that. I will say, what you call a messy work around is actually my preferred organization. I've always defined one class, one struct, one schema per file and it just feels more natural to me than defining an entire data model in a single file (although I concede XML kind of endorsed that kind of behavior).

That said, there are a few things I noticed about your schema:

  • As #1107 points out, it can be difficult to guess which package a type will be created in when that type doesn't have have an explicit package declared.
  • Your use of "existingJavaType": "java.util.Map<String, MessageAttribute>", does not fully qualify the MessageAttribute type, which could result in a missing import if the MessageAttribute is not in the same package as your Message type, which appears to be named implicitly by the file name as opposed to being explicitly declared.
  • My understand of existingJavaType is that it's for use when the declared type already exist on the classpath at the time generation is occurring. In your case, the class you're trying to use has not yet been defined, particularly if we were to process definitions after their enclosing class (for example if they were being created as InnerClasses).

dewthefifth avatar Apr 09 '20 21:04 dewthefifth

When jsonschema2pojo was created, 'definitions' was not part of the standard. Many people started using 'definitions' to hold extra schemas but we didn't add support here because it was just a common pattern but not part of the standard.

Now it has been brought into the standard I don't have a problem with adding support for it. Over the years a lot of people have expected it to work (and it appears that many people have a schema document with only definitions, and no root schema to reference them).

I'd probably just bump to 1.1.x for this change.

joelittlejohn avatar Apr 10 '20 07:04 joelittlejohn

I'll take a look at it this weekend and try to get an implementation. I haven't tinkered in those parts of the code before, so I'm not sure what the LOE will be, but don't mind getting my hands dirty.

That said, where do we envision the generated classes being generated?

  • As inner classes of the defining schema
  • As their own classes in the same package as the defining schema
  • In a package named after the defining schema, but in their own classes

I feel like inner class is the way to go because you could have multiple schema with definitions using the same name, defined in the same package. I'm not sure how the JSON schema treats that behavior, or whether it's allowed. I'm not sure the JSON schema even discusses how schemas should relate to one another when not referencing one another.

If I generate them as inner classes, which makes the most sense to me, or in their own classes within the same package what should their visibility be?

  • private
  • protected
  • public
  • package default

I guess I'm putting forward the following suggesting and looking for input:

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "type": "object",
  "additionalProperties": false,
  "definitions": {
    "MessageAttribute": {
      "type": "object",
      "additionalProperties": true,
      "properties": {
        "Type": {
          "type": "string"
        },
        "Value": {
          "type": "string"
        }
      }
    }
  },
  "properties": {
    "MessageAttributes": {
      "existingJavaType": "java.util.Map<String, MessageAttribute>",
      "type": "object",
      "additionalProperties": { "$ref": "#/definitions/MessageAttribute" }
    }
  }
}

becomes

public class Message
{
  private static class MessageAttribute
  {
    private String type;
    private String value;
    private Map<String, Object> additionalProperties;
  }

  private Map<String, MessageAttribute> messageAttributes;
}

dewthefifth avatar Apr 10 '20 13:04 dewthefifth

I don't think we should use inner classes, this pattern isn't used anywhere else. It also creates a more serious breaking change. I think these should go into whatever package is currently in context when we're processing the enclosing schema.

joelittlejohn avatar Apr 10 '20 13:04 joelittlejohn

I'll implement them as classes inside the current context, and make the assumption that they will not conflict with the "definitions" of another schema in that same context.

IE I'm going to expect it to throw an exception if two schemas in the same context have competing definitions.

dewthefifth avatar Apr 12 '20 14:04 dewthefifth

@joelittlejohn I took a stab at implementing this on https://github.com/joelittlejohn/jsonschema2pojo/pull/1124 however the ExtendsIT unit test fails after my implementation because it looks like there's already a sub-implementation for definitions when the definition is referenced directly by an extends call from within the actual schema.

Specifically, ExtendsIT has the following schema which processes with the current unit test.

{
  "definitions": {
    "parent": {
      "type": "object",
      "properties": {
        "propertyOfParent": {
          "type": "string"
        }
      }
    },
    "child": {
      "type": "object",
      "extends": {"$ref": "#/definitions/parent"},
      "properties": {
        "propertyOfChild": {
          "type": "string"
        }
      }
    }
  },
  "type": "object",
  "properties": {
    "child": {"$ref": "#/definitions/child"}
  }
}

My first stab at an implementation used the Schema.deriveChildSchema to define the schema used for definitions classes, which worked for my implemented unit tests but broke the existing ExtendsIT class because the ExtendsIT unit test ends up generating an invalid schema path when it encounters {"$ref": "#/definitions/parent"}. I dug deeper and determined that the current implementation makes use of the #definitions/parent portion of the schema to generate sub-schemas from within a larger file. I adjusted my schema definition to use the same convention, and stopped getting the path does not exist exception. However, I am not getting an assertion error out of the ExtendsIT unit test, because it looks like Parent is being processed twice. The first processing appears to come from my new code, where the schema is identified as #/definitions/parent and the second processing appears to be #/definitions/child/extends when the child class is processed (again by my new code). The end result is that I end up with a Parent.java and a Parent__1.java.

I'm hoping you can provide some insight as to the specifics of why the child identifies it's extension as #/definitions/child/extends instead of as #/definitions/parent, and provide some guidance as to how you'd like the issue resolved.

For reference, the following PR is where my existing changes live: https://github.com/joelittlejohn/jsonschema2pojo/pull/1124

dewthefifth avatar Apr 12 '20 17:04 dewthefifth

Hello everyody,

is there a chance to get this done for 1.1.0 ?

rfelgent avatar Oct 09 '20 20:10 rfelgent

Any news on this, I have a file that looks like:

{
  "allOf": [
    {
      "$ref": "#/definitions/Check"
    }
  ],
  "definitions": {
    "Check": {
"description": "The model for a Check",
      "type": "object",
      "required": [
        "header"
      ],
      "properties": {

     }
      

EDIT: I managed to make it work by moving the referenced definition (Check) as the top level object. But it seems like this is quite standard and should work.

yelhouti avatar May 31 '22 08:05 yelhouti

Is there any plan to implement the feature to force generation of unreferenced parts declared in the #definitions using a configuration?

dkellenb avatar Nov 02 '22 17:11 dkellenb

This would be useful for us, as it would allow us to generate Java classes from the definitions in a swagger/OAS2 specification (which I don't believe is possible otherwise, without moving all those definitions into separate files...)

lejtemxviw avatar May 30 '23 04:05 lejtemxviw