Hi
On 07/11/16 22:52, J. Fiala wrote:
Hi Sergey,
I think we should split the whole thing into two topics (issues/PRs):
Well. I'd rather have a single approach working both ways.
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.
As I said I'm not keen on this particular approach because using an
incomplete XML schema fragments to implicitly carry the optional
parameter validation info, where any of this String/int parameters has
nothing to do with XML schema, look wrong to me. It does not scale.
2.) BeanValidation for the WadlGenerator and for SourceGenerator for
easier roundtrip development
It is not the easier roundtrip dev but about having a complete solution
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.
I honestly would not like to go into the whole XML schema for the
purpose of optionally adding some bean validations, we've spent a lot of
time talking about it.
For wadl:param:
For @NotNull I'd prefer to use wadl:param@required, no need for the
custom namespace here.
Sure start with supporting NotNull.
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>
That won't scale once you'd want to add one more annotation support. It
would have to be something like
<cxfrs:beanValidation>
<cxfrs:size minLength="1" maxLength="100" />
</cxfrs:beanValidation>
without any nested minLength/etc to simplify the processing of 'size'
and to support adding of other beanVal, including NotNull, ex:
<cxfrs:beanValidation>
<cxfrs:size minLength="1" maxLength="100" />
<cxfrs:notNull/>
</cxfrs:beanValidation>
It is better to have a single place for everything related to beanVal.
SourceGenerator: look for nested beanValidation tag and pick validations
up.
yes
After implementing this, the next step would be to support the custom
tag for the complexTypes/JAXB-processing...
I'm sorry I disagree. Lets keep XMLSchema out of the picture for now as
far as the validation of optional parameters is concerned
Cheers, Sergey
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
