Hi Sergey,

I think we should split the whole thing into two topics (issues/PRs):

1.) SourceGenerator for contract-first-only with 100% portability
This should be supported for people who only practice contract first and don't care of the WadlGenerator and simply want to use clean simple/nested types for their WADL parameters. This warrants 100% portability for the WADL contract.

a) support simpletypes:

<param name="username" style="query" type="myns:username" required="false" />

<xs:simpleType name="username">
        <xs:restriction base="xs:string">
            <xs:maxLength value="50"/>
        </xs:restriction>
    </xs:simpleType>

This is what I already implemented, but only for the directly resolved type (like example here), here I could go deeper as well to further restrictions (<xs restriction base="mydeepersimpletype"...>).

b) support nested types:

<param name="username" style="query" type="myns:username" required="false" >
<xs:simpleType>
<xs:restriction base="xs:string">
<xs:maxLength value="50" />
</xs:restriction>
</param>

This is what I already implemented (check if nested simpleType is present + pick it up).

If you like to I can rework the PR https://github.com/apache/cxf/pull/146 and add only the implementation of SourceGenerator so you can merge conveniently.

2.) BeanValidation for the WadlGenerator and for SourceGenerator for easier roundtrip development

I think the main task should be to pickup the beanvalidation annotations for both wadl:param AND xs:element/xs:complexTypes. If we only add support for parameters and don't support it in the complextypes it is only the minor part of the deal. However, with the apporach to get working what is possible without big effort, we can in first place only support wadl:param.

For wadl:param:
For @NotNull I'd prefer to use wadl:param@required, no need for the custom namespace here. So I can start with @Size (minLength/maxLength) here and setup a first implementation like the following.

WadlGenerator:
<wadl:param name="username" required="true">
<cxfrs:beanValidation>
  <cxfrs:minLength value="1"/>
  <cxfrs:maxLength value="100" />
</cxfrs:beanValidation>
</wadl:param>

SourceGenerator: look for nested beanValidation tag and pick validations up.

After implementing this, the next step would be to support the custom tag for the complexTypes/JAXB-processing...

If you agree with that, I suggest setting up two separate Jira issues so we can move all the further comments there.

What do you think of this idea?

Best regards,

Johannes



Am 07.11.2016 um 22:12 schrieb Sergey Beryozkin:
Perhaps you can start with supporting a not null only ?
If a parameter @required is set (this can be enabled on the WADL to Java if BeanVal NotNull is set) then SourceGenerator can add this annotation back if its -beanValidation is set. The only restriction is that please use String class names of the BeanValidation for now until we figure our a neater approach to avoid the strong BeanVal dependencies

Cheers, Sergey
On 07/11/16 21:08, Sergey Beryozkin wrote:
Hi Johannes

Option 2 already works in the WADL to Java case. The only problem with
it, as discussed earlier, is that WADL grammar is meant to represent the
data representations, but SourceGenerator does not enforce it right now
and referring from the parameters to the schema elements is still
supported.

IMHO it makes sense to have a solution which works both ways first
without doing a massive effort as we don't really have time/scope for it.

Completing Option2 support will only require adding the parameter bean
classes to the JAXB context.
Option 1 will require a bit more work at the start but once you have
something like
<wadl:param>
<cxfrs:beanValidation>
  <notNull/>
  <someOtherBeanValName a="b" "c"="d">
</cxfrs:beanValidation>
</wadl:param>

then it will become quite straigtforward on the WADL to Java side...

Ideally one would even allocate a namespace which would extend the core
WADL namespace. Ex,

http://wadl.dev.java.net/extensions/bval

Might be worth investigating if it is allowed or not.

Cheers, Sergey

On 07/11/16 18:01, J. Fiala wrote:
Hi Sergey,

Thx for the analysis.

Regarding the WadlGenerator I'd also prefer option 1 opposing to option
2 (without the original idea available).

Regarding the SourceGenerator I think it should support clean parameter
type references in any way. So even if we follow option 1, we would need
to support correctly referenced simpletypes from parameters as well as
the cxfrs:beanValidation extension.

But probably it's easier to discuss this with examples together :).

Best regards,
Johannes


Am 07.11.2016 um 18:13 schrieb Sergey Beryozkin:
Hi Johannes

I've looked through various messages and pull requests.

One approach was to insert incomplete schema fragments into wadl:param
with the implicit understanding it was meant to create BeanVal
parameters and I felt quite uncomfortable about pursuing that
particular path.

The last idea we were trying to narrow upon was using a separate XML
schema grammar which would be used to create Bean validation if a bean
validation flag is set. This is a bit more WADL friendly but again it
does not look to me like a solution which says "this is to support the
bean validation" for parameters and I'm finding it difficult to see
how it will work both ways.

I'd to revisit these 2 proposals

1. have a CXF WADL extension, ex, cxfrs:beanValidation. This will be
in wadl:param and will be optimized around supporting all whatever is
needed to generate BeanVal annotations and represent any BeanVal in
the Java to WADL case.
This works both ways and IMHO simpler and cleaner, instead of trying
to 'carry' the bean validation info via XML schema fragments, with all
the limitations which seems ultimately wrong to me.
cxfrs:beanValidation will be of course only recognized by a CXF
wadl-to-java tool but I feel this approach will have more chances to
act getting more support via other provider's tool extensions than the
pseudo XML schema approach which is in fact would be equally CXF
specific.

2. Do not do anything at all but have wadl:param refer to XML complex
types. As I said earlier, this will require the use of
ParamConverterProvider which will have a chance to validate the beans.
These beans will already have BeanVal annotations in them. This also
works both ways - on the WADLToJava side you'd just add these bean
classes to a JAXBContext for the JAXB schema gen to add the relevant
schema definitions.

My preference is option 1).

We can discuss the options next week too,

Thanks, Sergey

On 06/11/16 22:25, Sergey Beryozkin wrote:
Hi Johannes

Can you please briefly summarize the proposed changes to WADLGen &
SourceGen ? There've been many emails and I recall I thought more work was needed to keep both approaches in sync. Right now I see WADLGen has
some explicit BeanVal dependencies. I def remember objecting to this
particular approach.
So how this can be generalized, be WADL friendly, and work both ways ?

Sergey
On 06/11/16 21:59, J. Fiala wrote:
Hi Sergey,

OK, I agree 100%, it would be great to have some progress with the PR
here if you have time for it.

Best regards,
Johannes


Am 06.11.2016 um 22:58 schrieb Sergey Beryozkin:
Hi Johannes

I'd say the WADL-related updates should be limited to
WADLGen/SourceGen

Cheers, Sergey
On 06/11/16 21:46, J. Fiala wrote:
Hi Sergey,

I can ask him or simply do a fork of the complete project and then
cover
it under the Apache license provided the author agrees with this.

Do you think further BeanValidation extensions for CXF should
also go
there and I should subclass the WadlGenerator/SourceGenerator? Or
shall
we merge this at the Hackathon at ApacheCon maybe?

Best regards,
Johannes


Am 06.11.2016 um 22:40 schrieb Sergey Beryozkin:
Hi Johannes

Would it make sense asking the author to give you the commit
rights to
the project ?
IMHO it can't go to the CXF master though perhaps an option of
opening
a CXF sub-project is viable which will need to be discussed on
the CXF
dev list. Whether it will stay where it is now or end up as a CXF
project, you'd have to support and release it, so I'm not sure what
difference would it make if it ended up being a CXF sub-project
given
it already has a home...

Thanks, Sergey


On 06/11/16 19:32, Johannes Fiala wrote:
Hi there,

The JAXB annotation plugin provided by krasa
(https://github.com/krasa/krasa-jaxb-tools) is no longer
supported,
does
it make sense to contact the author to merge this into CXF master?

Maybe we could also get the BeanValidation for parameters working
(https://github.com/apache/cxf/pull/146/files) to improve the
overall
BeanValidation experience.

Is anybody interested in putting this on the agenda for the
hackathon?

Best regards,
Johannes















Reply via email to