Re: oaj.lib.Closeable

[Stephen Allen <sa...@apache.org>]

On Wed, Apr 30, 2014 at 5:58 AM, Andy Seaborne <an...@apache.org> wrote:

> On 30/04/14 00:41, Stephen Allen wrote:
>
>> On Tue, Apr 29, 2014 at 5:43 PM, Andy Seaborne <an...@apache.org> wrote:
>>
>>  On 29/04/14 20:29, Stephen Allen wrote:
>>>
>>>  One simple change I would really like to make when we switch to Java 7
>>>> is
>>>> to make org.apache.jena.atlas.lib.Closeable extend
>>>> java.lang.AutoCloseable.
>>>>    That way we can take advantage of try-with-resources.
>>>>
>>>>
>>> Which classes/patterns in particular did you want autoclosability?
>>> org.apache.jena.atlas.lib.Closeable gets used widely.
>>>
>>> oaj.lib.Closeable defines close without a checked exception;
>>> java.lang.AutoCloseable defines close to throw an exception:
>>>
>>>
>>>  I want all of them to implement AutoCloseable.  Then the user can use
>> try-with-resources on any of our oaj.lib.Closeable classes.  We would have
>> oaj.lib.Closeable extend java.lang.AutoCloseable and override the close()
>> method to throw no exception.  That doesn't break any legacy interaction,
>> but adds the try-with-resources feature if you want it.  See the last
>> section of [1] that describes this behavior.
>>
>
> I took a look at everything from oaj.lib.Closeable.
>
> It would be helpful on many things (and documentation needs changing for
> the publicly visible items).
>
> For some things it's not helpful (e.g. TDB Journal) and where we don't
> require closing (TDB TupleTables - looks more like it simply does not need
> Closeable as it's meaning less history).  Several internal items (e.g.
> QueryEngineBase) don't need Closeable either.  These are no used in the
> try-with-resources fashion so adding AutoCloseable implies the wrong thing.
>
> An extreme case of DatasetGraph where "close" has specific meaning. This
> could have Closeable removed.
>
>
Great, clean up is always a good idea.



> I got ~30 warnings
>
>
Interesting, were these warnings where the Closeable object wasn't actually
closed?



> Stephen - you have been working on an alternative API - how do you want to
> advance that?
>

My company has been using it for the past year or so, and it seems quite
stable and useful as a SPARQL-based API.  Additionally, it is really useful
to have the same interface for local and remote connections, as it makes
unit testing a lot easier.

I have a two main things I would like to address before it is released, so
targeting a post Java 7 version of Jena makes sense:

1)  We are starting a new project that requires remote transactions, so
I've been working on a specification for extending the SPARQL HTTP protocol
to allow that.  Once I have a good draft of that, hopefully it won't be too
hard to implement in Fuseki2 (famous last words).

2)  I want to change the name of the general top level object in
jena-client from "Repository" to "DatabaseClient".  I have a secret plan to
also at one point add a new organizational structure to Jena above the
Dataset level called "Database".  This would contain 1 or more Datasets,
with the potential to perform locking across them or just on a single
dataset (think RDBMS schemas).  One of the main reasons I would like to do
this is to provide a location to store metadata (such as TDB's query
statistics, which would allow online statistics updating for the
optimizer).  The user could also use this to query the internals of the
database (see Postgres' System Catalogs [1]).  (Note that this last part
may be way in the future).

-Stephen

[1] http://www.postgresql.org/docs/9.3/static/catalogs.html

Re: oaj.lib.Closeable

[Andy Seaborne <an...@apache.org>]

On 02/05/14 01:55, Stephen Allen wrote:
> On Wed, Apr 30, 2014 at 5:58 AM, Andy Seaborne <an...@apache.org> wrote:
>
>> On 30/04/14 00:41, Stephen Allen wrote:
>>
>>> On Tue, Apr 29, 2014 at 5:43 PM, Andy Seaborne <an...@apache.org> wrote:
>>>
>>>   On 29/04/14 20:29, Stephen Allen wrote:
>>>>
>>>>   One simple change I would really like to make when we switch to Java 7
>>>>> is
>>>>> to make org.apache.jena.atlas.lib.Closeable extend
>>>>> java.lang.AutoCloseable.
>>>>>     That way we can take advantage of try-with-resources.
>>>>>
>>>>>
>>>> Which classes/patterns in particular did you want autoclosability?
>>>> org.apache.jena.atlas.lib.Closeable gets used widely.
>>>>
>>>> oaj.lib.Closeable defines close without a checked exception;
>>>> java.lang.AutoCloseable defines close to throw an exception:
>>>>
>>>>
>>>>   I want all of them to implement AutoCloseable.  Then the user can use
>>> try-with-resources on any of our oaj.lib.Closeable classes.  We would have
>>> oaj.lib.Closeable extend java.lang.AutoCloseable and override the close()
>>> method to throw no exception.  That doesn't break any legacy interaction,
>>> but adds the try-with-resources feature if you want it.  See the last
>>> section of [1] that describes this behavior.
>>>
>>
>> I took a look at everything from oaj.lib.Closeable.
>>
>> It would be helpful on many things (and documentation needs changing for
>> the publicly visible items).
>>
>> For some things it's not helpful (e.g. TDB Journal) and where we don't
>> require closing (TDB TupleTables - looks more like it simply does not need
>> Closeable as it's meaning less history).  Several internal items (e.g.
>> QueryEngineBase) don't need Closeable either.  These are no used in the
>> try-with-resources fashion so adding AutoCloseable implies the wrong thing.
>>
>> An extreme case of DatasetGraph where "close" has specific meaning. This
>> could have Closeable removed.
>>
>>
> Great, clean up is always a good idea.
>
>
>
>> I got ~30 warnings
>>
>>
> Interesting, were these warnings where the Closeable object wasn't actually
> closed?

They were being closed but not in the method creating the resources so 
they don't fit the try-with-resources pattern.  A Closeable resource is 
created and either passed to another method or returned from a method.

The iterator for a query being one of these!

I think we need to distinguish the two cases.  The API has 
try-with-resource patterns; the implementation of that does not.

	Andy