search

phoenixnap / springmvc-raml-plugin

Spring MVC - RAML Spec Synchroniser Plugin. A Maven plugin designed to Generate Server & Client code in Spring from a RAML API descriptor and conversely, a RAML API document from the SpringMVC Server implementation.
Apache License 2.0
136 stars 84 forks source link

Add Support for RAML 1.0 #1

Closed kurtpa closed 6 years ago

kurtpa commented 8 years ago

Add support for generation and parsing of RAML 1.0 documents

kurtpa commented 8 years ago

This might be more work than expected since the parser looks quite different for 1.0

kurtpa commented 8 years ago

De prioritized for now

aweisser commented 8 years ago

Do you have any idea how a migration path from 0.8 to 1.0 support could look like?

kurtpa commented 8 years ago

The biggest issue is the lack of support for 1.0 in the current raml parser. Last I checked the 1.0 parser uses a vastly different internal structure. The idea would be to upgrade parser first leaving support for 0.8 and then add in the new features from 1.0

aweisser commented 8 years ago

I analyzed the dependencies to org.raml (java-raml-parser) in springmvc-raml-plugin.

So here's my approach:

I'd decouple the springmvc-raml-parser from the third partyjava-raml-parser module by introducing an abstraction layer (interfaces) and adapters tojava-raml-parser v1 and v2 (raml0.8) and later v2 (raml1.0).

For additional raml 1.0 features we can use extensions to the generic interfaces. This way we can leave the existing code as is with a high refactoring safety and robustness.

I'm about to push an example of how this refactoring might look like.

What do you think?

kurtpa commented 8 years ago

Sounds awesome :) I'll owe you many pints of beer if you pulk this off :)

aweisser commented 8 years ago

I already pushed something to my fork (feature/#1) branch). Not sure why it's not linked here ...

aweisser commented 8 years ago

I started implementing an abstraction layer for the raml.model stuff so that we can adapt different raml parsers. So far I introduced interfaces like

This should be a real refactoring, so I provided implementations for those interfaces based on the current raml-java-parser (v1 for Raml Spec v0.8). I'm not finished with this refactoring. There a still dependencies to org.raml.model package that needs to be abstracted. But this is somehow trivial. ;)

So as a next step towards a proof of concept I started to provide an implementation for the model interfaces (RamlRoot, RamlResource, RamlAction ...) for java-raml-parser (v2 for Raml Spec v0.8). And now it get's interesting.

It seems that the new version does not support the serialization of the model as .raml file... I did not find any equivalence to RamlEmitter for example. It would be extra work to provide a RamlEmitter v2...

I raised an issue at raml-java-parser project (https://github.com/raml-org/raml-java-parser/issues/159).

kurtpa commented 8 years ago

I think once the abstraction layer is done we should merge it back into Master so that future development will be in line with the direction you're headed and we won't have refactoring work further down the line.

kurtpa commented 7 years ago

@aweisser not sure if you are still working on this, however the RamlEmitter we were working for seems not to be on the roadmap for 1.0 :( http://forums.raml.org/t/raml-to-springmvc/735/24

kurtpa commented 7 years ago

but in the issue you raised it seems like they are in fact working on it :) miscommunication i guess :)

aweisser commented 7 years ago

Yes. Issue #159 seems to be on the roadmap for 1.1.0. But there's is another issue (#201) that's a blocker somehow.

I guess we could implement workarounds for either. But it would be extra work. I think it's better to contribute to raml-java-parser project instead. Unfortunately I can't spent much time right now.

aweisser commented 7 years ago

Beyond that I'm not sure what else is missing to make springmvc-raml-plugin work with raml-java-parser v2.

The last time I worked on this I tried to add RAML v.0.8 support with raml-java-parser v2. I created a branch in my fork.

My original plan was to finish the RAML 0.8 support with the new raml-java-parser and then add RAML 1.0 support.

But I'm currently stuck, as mentioned above...

kurtpa commented 7 years ago

ya no worries :) let's wait for the good folk at raml parser to do their thing and i'll see if i can help there

kurtpa commented 7 years ago

Another option would be to keep using the old parser for 0.8 and use the new parser for 1.0, and only use the supported paths. Ie we would not support Code-to-RAML for 1.0 until they complete #159 at least

aweisser commented 7 years ago

Sure. Using the old parser for 0.8 is ok. My guess was that supporting 0.8 with the new parser would be a subset of implementing 1.0 support, because the new parser works in a complete different way for 0.8 and 1.0 spec. You stated that before... So as a clean and robust migration path I'd recommend to add 0.8 support with the new parser first. Or do you think that 1.0 support is easier to achieve directly?

kurtpa commented 7 years ago

if 0.8 works with the 0.8 parser, it could be "safer" to leave it as is. With the way you organised the code, it should be possible to package each parser and adapter classes in their own module. Like that at runtime we can enable 0.8 or 1.0 support as needed. In the beginning anything that 1.0 parser does not support will just return an operation unsupported exception and users can opt to use the 0.8 parser instead.

This will allow us to phase in 1.0 support slowly as the parser mature, and also without affecting the current functionality

What do you think?

aweisser commented 7 years ago

So you suggest some kind of alpha state for 1.0 support that updates incrementally with the state of raml parser while still providing fully functional 0.8 support using the old parser. That's indeed a possible way to go. But we still have to ensure a minimal but reasonable set of functions able to cope with at least the "standard" cases. Or we split it up vertically and try to support raml2spring with 1.0 first. This way we could ignore a missing RamlEmitter for now.

kurtpa commented 7 years ago

Exactly, I would split it vertically by flow - maybe start with raml2spring first as in my opinion it's the most important one (at least internally) and then we can continue the raml verification and spring2raml after.

aweisser commented 7 years ago

Ok. Kewl. I'd love to dive into it but I've very few resources atm. So I think it does not make sense if I open a branch in my fork repo. Could you open a branch im the base repo that we can collaborate on? Perhaps I can go the first few steps in the next couple of days...

We will definitely raise raml-parser issues or change requests as we go. Do you have good connections to the raml-parser folks to speed things up? ;)

kurtpa commented 7 years ago

https://github.com/phoenixnap/springmvc-raml-plugin/tree/feature/enable-raml-1.0-support

Branch created :) feel free to commit anything that isnt complete and we can move together there (since refactoring may make it difficult otherwise)

RE java parser.. i can offer to send them beers :D not sure if it will help :D

aweisser commented 7 years ago

Ok. Before hacking into it I started reasoning about a reliable testing strategy. Here are some thoughts:

  1. RAML 1.0 is not downwards compatible to version 0.8 of the spec. So we can't write simple equivalence tests based on our existing tests using two different implementations of the com.phoenixnap.oss.ramlapisync.raml interfaces.
  2. We don't need to test the whole process from a given .raml file to the generated Java code model. It'd be sufficient to parse a .raml file and verify the com.phoenixnap.oss.ramlapisync.raml model. This way we make sure that the new raml-java-parser output can be transformed to our internal model.
  3. We should take special care for the breaking changes of RAML 1.0
  4. We should carefully scan the whole RAML 1.0 spec and decide which of the new parts should be reflected by our internal com.phoenixnap.oss.ramlapisync.raml model and how those additional parts extend our code generation rules. We then should provide explicit tests for the extensions.

Note that this only reflects the first vertical part raml2java. I did not reason about the java2raml increment yet.

@kurtpa : any additions or comments?

kurtpa commented 7 years ago
  1. I agree, i think this adds value to the idea that we are going to separate the flows/modules for 0.8 and 1.0 with the understanding that not everythign will work for every version. As such i think it's important that we document this in our code
  2. Agreed
  3. Agreed
  4. Agreed, as i mentioned, we should also list changes to the model that are 1.0 specific
sichvoge commented 7 years ago

Hi. you might want to have a look at https://github.com/OnPositive/aml. There is good things in there as well as a RAML emitter as it seems.

kurtpa commented 7 years ago

With the latest commits on the the feature branch (https://github.com/phoenixnap/springmvc-raml-plugin/commit/40649fb1a8f7126cee5258f940b73b39bfce7674) we have initial support for RAML to Code 1.0. Please treat this as experimental and report any issues you might find in this thread.

Thanks to @aweisser for grabbing the bull by the horns and starting this off and to @stojsavljevic

DevWurm commented 7 years ago

Could you provide a snapshot version in a Maven repository, so it can be tested easily?

kurtpa commented 7 years ago

We're finalising some internal testing to ensure that old code paths continue to behave as per 0.9.1 and i will be releasing 0.10.0 to central in the coming days. When we do, RAML 1.0 Support will be experimental ;)

kurtpa commented 7 years ago

0.10.0 released to central, please use with care :)

DevWurm commented 7 years ago

Awesome, thank you! Is it already possible to generate RAML 1.0 files from my sources?

kurtparis commented 7 years ago

No sorry, for now only the reverse is possible. Next step :)

DevWurm commented 7 years ago

No reasons to be sorry for, you do great work! 👍 Could you add an issue for this feature, so e can keep track about it?

kurtparis commented 7 years ago

I am not closing this issue until raml 1.0 support is fully implemented for all code paths :)

DevWurm commented 7 years ago

Ah Ok :)

jenssaade commented 7 years ago

Kurt, can you check on the array type when generating? Seems the import statement for java.util.List/Set is missing after POJO creation. (running 0.10.0 from master)

kurtpa commented 7 years ago

Good morning @jenssaade - where is the import missing please - in the POJO itself or controller? I'm assuming you are running on a RAML 1.0 file?

jenssaade commented 7 years ago

Seems to miss in the POJO itself, opened #128 with details. I could not figure out how to fix this.

iyerajesh commented 7 years ago

@kurtpa I was trying to generate RAML 1.0 from my Spring MVC code. Is that supported with version 0.10.0?

kurtpa commented 7 years ago

There is 0.10.2 that fixes a number of issues, and 0.10.3 with more improvements coming out soon

iyerajesh commented 7 years ago

I tried 0.10.2, it still generates RAML 0.8

kurtpa commented 7 years ago

Oh, no sorry we only support RAML 1.0 when generating Java from RAML, not the other way around

iyerajesh commented 7 years ago

is the other way around, coming soon? Is there any way I could contribute towards that work?

kurtpa commented 7 years ago

It's on our to-do list but you'd be very welcome to contribute

ghost commented 7 years ago

It would be greatly appreciated if you would have the kindness to take few minutes to mention the fact that 1.0 is not supported from java to raml in your documentation page.

Thanks,

J

champagne commented 7 years ago

Hi @kurtpa ,

I'm a Java developer. I'm very interested in this plugin and I'd like to do something contribution. Where could I start from?

BR, Xiangbin HAN

stojsavljevic commented 7 years ago

@champagne

Thank you for your interest. Can you please take a look at #154?

Thanks again!

iyerajesh commented 7 years ago

Hi Alexander,

Iam a java architect and developer. I am very much interested in contributing and enhancing this plugin. Where can I start?

Thank you, Raj On Thu, Aug 31, 2017 at 9:29 AM Aleksandar Stojsavljevic < notifications@github.com> wrote:

@champagne https://github.com/champagne

Thank you for your interest. Can you please take a look at #154 https://github.com/phoenixnap/springmvc-raml-plugin/issues/154?

Thanks again!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/phoenixnap/springmvc-raml-plugin/issues/1#issuecomment-326295494, or mute the thread https://github.com/notifications/unsubscribe-auth/AHicZP9OWc8GYLrEp1LdELd0L6R3CcKnks5sdrVUgaJpZM4GzkaJ .

stojsavljevic commented 7 years ago

What a community 🥇

@iyerajesh can you a take a look on #163?

champagne commented 7 years ago

@stojsavljevic , I'll take it, #154 .

jpbelang commented 7 years ago

I was wondering if any of these two projects could be of a use to you:

https://github.com/mulesoft-labs/raml-java-tools/tree/master/raml-builder

This project allows you to build a RAML model by hand with some validation.

and

https://github.com/mulesoft-labs/raml-java-tools/tree/master/raml-simple-emitter

This emits a RAML model.

I'm using them in my raml-for-jaxrs project. There is still some work to do on them, but I have more work done than to do.

Thanks.

stojsavljevic commented 7 years ago

Hi @jpbelang

I'm in contact with @jstoiko and that is exactly our plan :)