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

Thu Jul 5 15:30:54 EDT 2012

Hmm, the foregoing discussion still sidesteps the main subject of this
thread.  Aren't we saying that the current API implies this workflow:

* user calls Configuration#addMapping() 0..n times
* user calls Configuration#buildValidatorFactory()
* Configuration passes ConfigurationState to
ValidationProvider#buildValidatorFactory()
* ValidationProvider:
** queries ConfigurationState#getMappingStreams()
** sets up result for Configuration#getConfigurationSource()
** returns ValidatorFactory

?  This seems tangled up to me, compared to having the Configuration
itself handle the task of assembling ConfigurationSource from the
streams and having the ValidationProvider configure itself from the
ConfigurationSource.  If we're really just worried about API
backward-compatibility, it might not be that hard to create on-demand
a validation XML stream that represents the cumulative (from
potentially multiple XML streams) ConfigurationSource in case of any
ValidationProviders that want to read the raw XML themselves.

Matt

On Thu, Jul 5, 2012 at 9:50 AM, Matt Benson <mbenson at apache.org> wrote:
> On Wed, Jul 4, 2012 at 4:37 PM, Gunnar Morling <gunnar at hibernate.org> wrote:
>> 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?
>>
>
> Okay, strike my last comment.  "passed programmatically in
> Configuration" is the reference to Configuration#addMapping() in
> ConfigurationState#getMappingStreams(), and I must simply have missed
> it before.  The beginning part of my statement addressed Hardy's "it
> is also a general programmatic configuration source to configure Bean
> Validation" by which I had understood him to imply that these methods
> do *not* necessarily refer to validation XML configuration.  The
> existing javadoc for each of the methods in question seems very
> clearly to dictate that the content available be validation XML
> configuration, hence my confusion.  Perhaps I mistook his meaning,
> however.
>
>>>
>>>> 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.
>
> Of course the old #addMapping() mechanism must be supported.
> #getMappingStreams() could continue to be the access point.  This
> would make it possible to draw a clear line in the sand: a BV
> implementation does not have to support multiple
> #buildValidatorFactory() calls with externally specified InputStreams,
> but must do so for DataSources whose #getInputStream() methods can be
> called multiple times.  Wrapping an InputStream to an anonymous
> DataSource would of course be trivial and would allow the bulk of
> handling to be done by a single codepath.
>
>>
>> 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.
>
> As far as the repeatability question goes:  the javadoc for
> Configuration#addMapping() already states that the client is
> responsible for closing InputStreams added via this mechanism;
> assuming that we do not agree to using DataSource, would it be more
> appropriate to make the client responsible for any repeatability here
> as well?
>
> Matt
>
>>
>> --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
>> _______________________________________________
>> beanvalidation-dev mailing list
>> beanvalidation-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev