I would also vote for option 2. All changes sound good to me.
Waiting for the patches then
I´ve been working on this issue and I've some question/doubts:
If find() methods from AuditReader are capable of retrieving the objects audited by a NOT_AUDITED relation (option 2 of my last post), the "throws NotAuditException" won´t be semantically correct anymore, because those methods would check the not_audited relationships before returning or throw a different exception in other case. If we pick up this solution, Envers users that are currently catching the NotAuditedException should do a code refactor; they should ask if the relationship is audited or not before using the method instead of catching the exception.
Another way to solve the problem and avoid the migration, is to add new findSomethingElse() methods to AuditReader. Then, users who need this feature should call them instead of the currend find() methods.
I prefer the last solution. Could someone suggest a proper name to those methods? Something better than >>findHistoricAndCurrent()
Another question is: I´m testing my changes on all active branches (trunk, 3.5 and the old 3.3). Does it make sense to give you the three patches?
Here is the patch for this issue. There are 3 files, one per branch.
I've created the tests for testing it
I decided to name the new methods findAll(), but let me know if you have a more suitable name.
I hope it will help.
I've taken a quick look, but it would be great if you could also add documentation for the Envers reference manual; preferably as a separate patch.
What do you think can be documented and where would be the correct place to put it?
I've never worked with hibernate documentation, besides building it using maven's goal. Where can I find what to use for edit the documentation?
The Envers refence manual is currently available at http://docs.jboss.org/hibernate/envers/3.6/reference/en-US/html_single/
Wouldn't it be good to expand chapter 5 with this new query api?
The manual is built from hibernate svn, look in /documentation/envers (if I'm not mistaken).
I think that que query section is not the correct place because this new methods are part of the find() methods family. Maybe there is a whole missing section in the documentation for "loading historical objects" if we want to give a good explanation.
What do you think?
Actually, you're right. I'm not sure though if this would require a new chapter, or that we count integrate all supported query methods in one chapter with several sections. Perhaps you could propose a chapter setup?
Taking a look at hibernate core's documentation, I saw that chapter 10 has the following structure:
10. Working with objects
10.1. Hibernate object states
10.2. Making objects persistent
10.3. Loading an object
10.4.1. Executing queries
10.4.2. Filtering collections
10.4.3. Criteria queries
10.4.4. Queries in native SQL
10.5. Modifying persistent objects
10.6. Modifying detached objects
10.7. Automatic state detection
10.8. Deleting persistent objects
10.9. Replicating object between two different datastores
10.10. Flushing the Session
10.11. Transitive persistence
10.12. Using metadata
Can we follow this idea and propose something like:
5. Working with historical objects
5.1 Loading a historical object (explain the find methods here)
5.2 Mapping a class more than once (explain support for entityNames)
5.4 Explain all query options for envers
What do you think?
Yes, that's exactly what I meant.
I would be great if you could create a patch for the documentation (against current trunk) and attach it to the issue .
Assuming you meant 5.3.1 Explain query options for envers (instead of 5.4)
You're right. I meant 5.3.1.
I will work on it and come back with the patch.
Sorry for jumping in so late, but there's one thing I don't quite understand - if there's a isEntityClassAudited/isEntityNameAudited, why do we need a findAll method? Users can check if an entity is audited or not and use and auditReader or an entityManager, or do I miss some use-case?