[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15805064#comment-15805064 ] Craig L Russell commented on JDO-735: - A14.6.7-2 [This method closes all results of execute(...) methods on this Query instance, as above. The Query instance is still valid and can still be used.] A14.6.7-3 Add the assertion here. > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java, JDO-735-patch.txt > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15805058#comment-15805058 ] Craig L Russell commented on JDO-735: - We need assertions for these test cases. So we need to update the specification to add the assertions. The text can be copied from the Extent and Query javadoc. > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java, JDO-735-patch.txt > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15696378#comment-15696378 ] Michael Bouschen commented on JDO-735: -- Need a tck test for try-with-resources for Extent, and Query. Maybe org.apache.jdo.tck.extents.AutoCloseable.java org.apache.jdo.tck.query.api.AutoCloseable.java The test class for PersistenceManager is pretty simple with no error cases. We should think about adding some cases where the test() method throws an exception and make sure the PM is closed: org.apache.jdo.tck.api.persistencemanager.close.AutoCloseable.java > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15637513#comment-15637513 ] Craig L Russell commented on JDO-735: - There are several competing interests here: 1. Dissuade programmers from using close(). They should use closeAll() instead. With autocomplete in their Java IDE, when they start to type clos they should get javadoc which should say "don't use this method; use closeAll instead". 2. If programmers accidentally use close() they should have to catch Exception, which is the only method in JDO that throws Exception. So we need to either *not* declare close() in the interfaces or declare close() with the throws Exception clause. 3. If we declare void close() throws Exception and say its semantics are identical to closeAll(), users might think "If semantics are identical, why does close() throw Exception and closeAll() does not"? The best solution seems to be: Declare (override) void close() throws Exception in the Query and Extent interfaces. Provide javadoc that says "don't use this method. It is intended for use by try-with-resources. Use closeAll() instead." > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15616051#comment-15616051 ] Tilmann Zäschke commented on JDO-735: - In theory, I could imagine that a database server process could benefit from a {{close()}} method. While {{closeAll()}} releases all resources associated with a specific query, {{close()}} could serve as a hint to the server that no similar query is expected to be executed in the near future. This may allow the server to drop resources such as index pages that were only used for that query. This would not necessarily require support for {{isClosed()}} (unusable state), because a user could still be allowed to use the query later on. The most resources are probably associated with query results, not queries. Therefore, an alternative may be to make query results AutoCloseable. This would require queries (such as the new JDOQLTypedQuery) to return something like a {{CloseableList}} when executed. > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1583#comment-1583 ] Craig L Russell commented on JDO-735: - I think the isClosed method only makes sense if there are resources associated with the object. In the context of Extent and Query, the only resources are the iterators and result sets. And if close and closeAll are idempotent, there is no reason for the isClosed method. If you want to free resources, call close or closeAll. And I don't think that close should make the object unusable. What value do we add by making the Extent or Query unusable? > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15429359#comment-15429359 ] Andy Jefferson commented on JDO-735: Perhaps Extent should also be AutoCloseable? Perhaps Query should have isClosed() added and throw JDOUserException if a method called when already closed (for consistency with PersistenceManager). And ditto Extent if the first question is affirmative. > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson >Assignee: Michael Bouschen > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14741106#comment-14741106 ] Michael Bouschen commented on JDO-735: -- Looks good. Should we keep the test class CloseThrowsExceptionWhenActiveTx.java in svn? I don't think so, so if everyboyd agrees I will remove it. > Make PersistenceManager and Query support AutoCloseable (JDK1.7+) > - > > Key: JDO-735 > URL: https://issues.apache.org/jira/browse/JDO-735 > Project: JDO > Issue Type: New Feature > Components: api, specification, tck >Reporter: Andy Jefferson > Fix For: JDO 3.2 > > Attachments: CloseWithActiveTxRollsBack.java > > > So then it can be used with JDK1.7+ try-with-resources, as per > http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14712782#comment-14712782 ] Andy Jefferson commented on JDO-735: Test now in SVN, along with another for AutoCloseable, and previous test is removed from pm.conf Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 Attachments: CloseWithActiveTxRollsBack.java So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14572482#comment-14572482 ] Andy Jefferson commented on JDO-735: Changing the spec will mean the TCK Test org.apache.jdo.tck.api.persistencemanager.close.CloseThrowsExceptionWhenActiveTx needs an update Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14547328#comment-14547328 ] Craig L Russell commented on JDO-735: - In a non-managed environment, if the current transaction is active, close throws JDOUserException. This could be interpreted as the state of the current transaction and of the pm remains the same. I'd like to at least see The transaction is rolled back and the PersistenceManager is closed. Aside from backward compatibility, is there any need to keep the throws JDOUserException? Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14547036#comment-14547036 ] Tilmann Zäschke commented on JDO-735: - +1 for rolling back active transactions on close() As I understand auto-closeable, it will rethrow the inner exception, i.e. the original exception that triggers the auto-close. Even if the close/rollback causes another exception, the original exception won't be lost. Should that not suffice? Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14546158#comment-14546158 ] Craig L Russell commented on JDO-735: - Oops. This is in 12.6: In a non-managed environment, if the current transaction is active, close JDOUserException. I'd like to revisit this in light of AutoCloseable. In order to make this serve its purpose, we need to be able to give the user a clear exception that reflects the application error and not the exception that the PersistenceManager was closed with an active transaction. Aside from making pm.close silently roll back the transaction and succeed, any other ideas? For backward compatibility, we could add a flag to PM but that is awkward. Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14545765#comment-14545765 ] Craig L Russell commented on JDO-735: - The specification is silent on the subject of PM.close() when a transaction is active. I think we need to add this behavior in light of auto-close. I'd think that if a transaction is active, close should roll back the transaction and complete normally. Consider this use-case: // this will either update the objects or throw an optimistic exception try (PersistenceManger pm = pmf.getPersistenceManager()) { pm.currentTransaction().begin(); // optimistic // do some updates to some objects pm.currentTransaction().commit(); } What we want to allow the user to do is to avoid all the work to figure out how to clean up the persistence manager in case of an error. Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537942#comment-14537942 ] Tilmann Zäschke commented on JDO-735: - Apologies for commenting so late. I see a small problem with making PersistenceManager auto-closeable: If I understand the spec (12.6) correctly, PM.close() will throw an Exception if an transaction is active. Since, in my experience, database exceptions mostly occur during open transactions, the auto-close will fail in most cases. One problem with this is probably that this will always swallow the actual error, only the failed close()-exception will be reported. A solution would be to change the spec such that calling close() on an open Transaction simply aborts() the transaction before closing it. I would prefer that anyway, because it would solve a separate problem that I usually have to do if-pm-is-active-then-abort-before-close in many catch blocks, which feels a bit like unnecessary boilerplate code. Any thoughts on this? I understand that this is a spec change, but I suppose it should be mostly backwards compatible. Also, I guess this should probably go into a separate issue. Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (JDO-735) Make PersistenceManager and Query support AutoCloseable (JDK1.7+)
[ https://issues.apache.org/jira/browse/JDO-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537966#comment-14537966 ] Tilmann Zäschke commented on JDO-735: - I just realised that auto-close will re-throw the inner exception, not the exception from a close() attempt. This leaves the discussion whether JDO should at least attempt to abort() a transaction before closing it. This may make things easier depending on the recovery mechanism of the database. Personally, I would find it useful if close() would abort a transaction, but I suppose that has been discussed before elsewhere in detail. Make PersistenceManager and Query support AutoCloseable (JDK1.7+) - Key: JDO-735 URL: https://issues.apache.org/jira/browse/JDO-735 Project: JDO Issue Type: New Feature Components: api, specification, tck Reporter: Andy Jefferson Fix For: JDO 3.2 So then it can be used with JDK1.7+ try-with-resources, as per http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html -- This message was sent by Atlassian JIRA (v6.3.4#6332)