On Wed, Jul 4, 2012 at 4:37 PM, Gunnar Morling <gunnar(a)hibernate.org> wrote:
Hi Matt,
2012/7/3 Matt Benson <mbenson(a)apache.org>:
> On Tue, Jul 3, 2012 at 4:02 AM, Hardy Ferentschik <hardy(a)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/m...
>> [3]
https://github.com/hibernate/hibernate-validator/blob/master/engine/src/m...
>>
>>
>> _______________________________________________
>> beanvalidation-dev mailing list
>> beanvalidation-dev(a)lists.jboss.org
>>
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev