mulesoft-labs / raml-for-jax-rs

This project is all about two way transformation of JAX-RS-annotated Java code to RAML API description and back.
Other
296 stars 181 forks source link

ramltopojo Rejects Perfectly Legal RAML Array Type Definitions #355

Closed deduper closed 6 years ago

deduper commented 6 years ago

Dev environment:


Running the attached RAML through raml-to-jaxrs-cli-3.0.3-20180623.232928-2-jar-with-dependencies.jar produces "org.raml.ramltopojo.GenerationException: unable to create type array item of type object (or maybe an inline array type ?)"

Whatever was done to resolve #337 seems to disallow perfectly legal datatypes with array properties from being generated.

For security reasons, my work is very paranoid about publicizing the RAML for our internal APIs. So I've renamed everything in the RAML I've attached here. But the general structure of everything is 100% identical to our actual internal API RAML.

jpbelang commented 6 years ago

I'll be looking at it this week. I need to finish the "additionalProperties" (should be done today).

Just so you know, the problem with this was not a change. The parser does not behave the same way and it breaks some stuff. However, the parser was updated, and it might work better.

I'll see if I can work out something otherwise. The bug that's problematic is here:

https://github.com/raml-org/raml-java-parser/issues/250

jpbelang commented 6 years ago

I have a plan: since the parser isn't helping at telling me whether a type is actually a new type, I'll have to add an annotation to notify raml-for-jaxrs that a type should be generated if it's inline. This will be done in raml-java-tools. This won't change the current behaviour for ObjectTypes (with properties) where I deduce that it's a new type that should be generated by checking if there are more properties declared.

It's what I'm working on now. I'm pushing back the release that I wanted to do in this weekend to get this through.

deduper commented 6 years ago

Thanks @jpbelang for looking into this. I look forward to your updates.

deduper commented 6 years ago

Hey @jpbelang

Howdy do? :) Any progress to share yet?

In the meantime, can you answer a couple general questions, please?

  1. The generation creates both interfaces and concrete implementation classes for the domain objects/types. What's the rationale behind not generating just the concrete implementation classes only; sans interfaces?
  2. The generation creates concrete Response classes (and others) as static class members of the generated Resource interfaces. What is the rationale for doing it that way? As oppose to generating them in separate/stand-alone top-level classes in their own right, for example?

For the record, I personally don't have any objection to the class structure created by the generation. But a couple of my team mates do. I'd just like to make sure that when I address their concerns and explain why the generation does what it does, I'm not putting words in your mouth.

Thanks in advance for your answers.

jpbelang commented 6 years ago

Yes, actually. The array stuff is working better in a more general way. Checkins are coming up. Still am not handling inline type (which is what you are using). I have a plan for that. Beginning of the week.

As to your questions:
1) The rationale is that objects need to support multiple inheritance. Resources need to support these polymorphically. As a general solution. That said, I've had on my plate to skip the generation of interfaces for applications who don't need it.

2) Version 1 of this project did it like that. It also becomes (unless I'm not understanding your question) a naming issue. And since responses are local to one resource (i.e. responses are defined in the resource in RAML, even though it might be dont through RAML "resourceType"s), I've always agreed with that idea.

jpbelang commented 6 years ago

Ok, I think I have you covered, with one caveat: you need to add the annotation for inline array types, otherwise you will get the same error. See the example at the bottom. (it's from here: https://github.com/mulesoft-labs/raml-java-tools/tree/release/1.0.3/raml-to-pojo-maven-example/src/main/resources)

The reason I need to do this is because of a bug in the parser: array types declared with the shortcut notation (eg: string[]) return exactly the opposite of the long form. So it does the reverse for the shortcuts: it always generates an inline type. So I have made it so that we only generate the inline type if the annotation is present and set to true.

types:
    mother:
      type: object
      properties:
       smaller:
          (ramltopojo.types):
            generateInlineArrayType: true
          type: array
          items:
            type: object
            properties:
              name: string
jpbelang commented 6 years ago

I put this in your:

#%RAML 1.0 Library
usage: The types associated with the domain of Cookery.
uses:
  ramltopojo: ../ramltopojo.raml
types:

  Chef: !include ../types/355.type-eats-request.raml

  Recipe: !include ../types/355.type-eats-response.raml

And added the annotation here:

#%RAML 1.0 DataType

description: What it takes to make it.
#type: object
properties:
  foods:
    (ramltopojo.types):
      generateInlineArrayType: true
    description: Foods used in a recipe.
    type: array
    items: !include 355.type-food.raml

And the build passed.

deduper commented 6 years ago

Hey @jpbelang !

Thanks for looking into that brother! And thanks for your answers to my questions. I will have a play around with that annotation and let you know how it works out for me.

I sincerely appreciate your help :thumbsup: