Re: [Libreoffice-commits] core.git: tdf#97597: attempt to add test for multithreaded input stream buffering.

> On January 18, 2017 at 3:31 AM Stephan Bergmann <sbergman@redhat.com> wrote:
>
>
> On 01/17/2017 03:06 AM, Kohei Yoshida wrote:
> > commit 294f2e627cc6f1d0483f7affcf96467a4bd3ba5a
> > Author: Kohei Yoshida <kohei.yoshida@collabora.com>
> > Date:   Mon Jan 16 15:33:37 2017 -0500
> >
> >     tdf#97597: attempt to add test for multithreaded input stream buffering.
> >
> >     But it always passes, even when UseBufferedStream is set to false...
> >     Needs improvement.
> >
> >     Change-Id: I98f65dcd7bec3b47a437fdc6cc42c6e8e3775522
> >     Reviewed-on: https://gerrit.libreoffice.org/33190
> >     Tested-by: Jenkins <ci@libreoffice.org>
> >     Reviewed-by: Kohei Yoshida <libreoffice@kohei.us>
> >
> > diff --git a/offapi/com/sun/star/packages/zip/ZipFileAccess.idl 
> > b/offapi/com/sun/star/packages/zip/ZipFileAccess.idl
> > index 6d54509..7677c94 100644
> > --- a/offapi/com/sun/star/packages/zip/ZipFileAccess.idl
> > +++ b/offapi/com/sun/star/packages/zip/ZipFileAccess.idl
> > @@ -23,7 +23,7 @@
> >  #include <com/sun/star/packages/zip/ZipException.idl>
> >  #include <com/sun/star/ucb/ContentCreationException.idl>
> >  #include <com/sun/star/ucb/InteractiveIOException.idl>
> > -
> > +#include <com/sun/star/beans/NamedValue.idl>
> >
> >
> >  module com {  module sun {  module star {   module packages {  module zip {
> > @@ -38,6 +38,12 @@ service ZipFileAccess : XZipFileAccess2
> >                   com::sun::star::ucb::ContentCreationException,
> >                   com::sun::star::ucb::InteractiveIOException,
> >                   com::sun::star::packages::zip::ZipException );
> > +
> > +    createWithArguments( [in] sequence<com::sun::star::beans::NamedValue> args )
> > +        raises ( com::sun::star::io::IOException,
> > +                 com::sun::star::ucb::ContentCreationException,
> > +                 com::sun::star::ucb::InteractiveIOException,
> > +                 com::sun::star::packages::zip::ZipException );
> >  };
>
> [...]
>
> > +
> > +        uno::Sequence<beans::NamedValue> aArgs(2);
> > +        aArgs[0].Name = "URL";
> > +        aArgs[0].Value <<= aURL;
> > +        aArgs[1].Name = "UseBufferedStream";
> > +        aArgs[1].Value <<= true;
> > +
> > +        uno::Reference<packages::zip::XZipFileAccess2> xZip(
> > +            
> > packages::zip::ZipFileAccess::createWithArguments(comphelper::getProcessComponentContext(), 
> > aArgs));
>
> Hm, I stumbled when seeing this commit.  Those "new-style" service ctors 
> are meant to make service instantiation easier and safer compared to the 
> original create-instance-from-factory way.  While createWithArguments 
> addresses safety in the return type, it does nothing for the arguments 
> (doesn't even document what they could be).

Can you elaborate on this?  Is it the lack of documentation you are concerned with (which I can 
gladly add), or what is lacking in terms of the "safety of the arguments"?

> For an apparently (at least for now) one-off usage, I'm not sure adding 
> such a ctor is too useful overall.  Maybe instead use the old 
> create-instance-from-factory way in that one place?

Well, I added this because I saw value in adding such a ctor.  So, at least one person finds it 
useful. :-)  If that's not enough of an argument for adding a new ctor, then so be it.

  (And if not 
> reverting it, please add at least a @since tag.)

Sure, that's no problem.

Kohei