[bv-dev] BVAL-265 - Expose settings defined in XML in the Configuration API

[Gunnar Morling]

Hi Matt,

2012/7/3 Matt Benson <mbenson at apache.org>:
> On Tue, Jul 3, 2012 at 4:02 AM, Hardy Ferentschik <hardy at hibernate.org> wrote:
>> Hi,
>>
>> comments inline
>>
>> On Jul 2, 2012, at 8:35 PM, Matt Benson wrote:
>>
>>> Hi all,
>>>  Sorry for my extreme tardiness in doing the backing research on this
>>> issue.  I am fine with the standing changes, so far as they go;
>>> however it seems like this could also be an opportunity of sorts.  If
>>> the Configuration object is going to expose the ConfigurationSource
>>> (and thereby the data corresponding to that mined from the XML
>>> configuration), it would seem to move the responsibility of parsing
>>> the XML and eliminate the need for ConfigurationState#getMappingStreams().
>>
>> I don't think that the changes introduced by ConfigurationSource have an effect on
>> ConfigurationState#getMappingStreams. Yes, this methods provides input streams to
>> the configuration files defined in validation.xml, but it is also a general programmatic
>> configuration source to configure Bean Validation.
>>
> I guess you must mean hypothetical implementation-specific
> configuration, but if that's the case it would seem the javadoc for
> Configuration#addMapping() and ConfigurationState#getMappingStreams()
> could/should be expanded--in any event, the latter should probably
> make some mention of the former.

Could you make a suggestion what you'd like to see clarified in the docs?

>
>> Whether it was/is a good choice to use input streams over strings (defining file paths)
>> is debatable. I think the idea was to allow other input sources than just files.
>> Generally I think this is a good idea,  but of course working with InputStream creates
>> another set of problems. Despite these problems I don't think we should change anything
>> regarding ConfigurationState#getMappingStreams(), since it servers a different purpose
>> than ConfigurationSource and changing it would break backwards compatibility.
>>
>> Also there are work arounds for the one issue you are referring to (see comment below).
>>
>>>  This method is AFAICT the
>>> place that complicates the building of multiple ValidatorFactory
>>> instances from a single Configuration, which was the subject of a
>>> thread which I cannot currently find, but whose participants I recall
>>> as having included Gunnar and myself
>>
>> I think you are referring to HV-563 ("Enable Configuration#buildValidatorFactory() to be callable several times").
>
> This is the issue (I think there was an email thread later on).

The original discussion was back in March (see
http://lists.jboss.org/pipermail/beanvalidation-dev/2012-March/000290.html).
There's also a BVAL issue for this
(https://hibernate.onjira.com/browse/BVAL-282).

>
>>
>> There was indeed an issue with building multiple ValidatorFactory instances from a single
>> Configuration instance (at least in Hibernate Validator). However, we found a way to solve this, by
>> checking whether the provided InputStream supports marking [2]. If it does than there is no problem and if it doesn't
>> we wrap the stream in a BufferedInputStream which does. Once you have a markable stream you can mark, read
>> and finally reset it [3]. This way you can reuse the Configuration multiple times.
>>
>
> Yes, I think I remember discussing this--not perfect, but serviceable.
>  Of course, if Bean Validation v1.1 were to move up to Java SE 6,
> javax.activation.DataSource might possibly be more appropriate as an
> intermediate type to *provide* an InputStream any number of times.
> Thoughts?

While discussing an issue with the spec JAR
(https://hibernate.onjira.com/browse/BVAL-298), Emmanuel agreed on
basing BV 1.1 on SE 6 (see
https://hibernate.onjira.com/browse/BVAL-299).

That said, I think j.a.DataSource indeed looks nice and I think it
would have been great if it could have been used in BV 1.0. But
introducing it now AFAICS basically means adding an overloaded method
like Configuration#addMapping(DataSource dataSource). As users might
still use the old method passing an input stream, we need to address
BVAL-282 anyways in order to make a configuration re-usable. But then
there is no much advantage by supporting j.a.DataSource IMO.

All in all I think fixing BVAL-282 should be good enough, with the
advantage of an unchanged API. On minor issue is the required
buffering of non-markable streams, but given the typical size of
mapping streams, I don't see that as real problem. And users may still
pass an appropriate stream implementation when working with huge
mapping configurations.

--Gunnar

>
> Matt
>
>>
>> --Hardy
>>
>>
>> [1] https://hibernate.onjira.com/browse/HV-563
>> [2] https://github.com/hibernate/hibernate-validator/blob/master/engine/src/main/java/org/hibernate/validator/internal/engine/ConfigurationImpl.java#L167
>> [3] https://github.com/hibernate/hibernate-validator/blob/master/engine/src/main/java/org/hibernate/validator/internal/xml/XmlMappingParser.java#L614
>>
>>
>> _______________________________________________
>> beanvalidation-dev mailing list
>> beanvalidation-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev