-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Docs: read-only parent's operations are still cascaded to its child associations #10772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1134,6 +1134,11 @@ Additionally, the `CascadeType.ALL` will propagate any Hibernate-specific operat | |||||
`REPLICATE`:: cascades the entity replicate operation. | ||||||
`LOCK`:: cascades the entity lock operation. | ||||||
|
||||||
[WARNING] | ||||||
==== | ||||||
When a parent entity is in a **read-only state**, operations are still cascaded to its child associations. For example, adding a new child to a collection on a read-only parent will result in the child entity being persisted. If this behavior is not desired, the parent entity needs to be evicted from the session before making modifications. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this section is not dealing with read-only mode at all, so this note is inappropriate in here. Please remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I mentioned in the ticket, the reason why I added it here is because it's literally the only place referenced by: Side notes:
|
||||||
==== | ||||||
|
||||||
The following examples will explain some of the aforementioned cascade operations using the following entities: | ||||||
|
||||||
[source, java, indent=0] | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -52,6 +52,8 @@ include::{example-dir-model}/PhoneType.java[tags=hql-examples-domain-model-examp | |||||
|
||||||
include::{example-dir-model}/Call.java[tags=hql-examples-domain-model-example] | ||||||
|
||||||
include::{example-dir-model}/Account.java[tags=hql-examples-domain-model-example] | ||||||
|
||||||
include::{example-dir-model}/Payment.java[tags=hql-examples-domain-model-example] | ||||||
|
||||||
include::{example-dir-model}/CreditCardPayment.java[tags=hql-examples-domain-model-example] | ||||||
|
@@ -452,7 +454,7 @@ See the https://docs.jboss.org/hibernate/orm/{majorMinorVersion}/javadocs/org/hi | |||||
[[hql-read-only-entities]] | ||||||
==== Querying for read-only entities | ||||||
|
||||||
As explained in <<chapters/domain/immutability.adoc#entity-immutability,entity immutability>>, fetching entities in read-only mode is more efficient than fetching entities whose state changes might need to be written to the database. | ||||||
As explained in <<chapters/domain/immutability.adoc#mutability-entity,entity immutability>>, fetching entities in read-only mode is more efficient than fetching entities whose state changes might need to be written to the database. | ||||||
Fortunately, even mutable entities may be fetched in read-only mode, with the benefit of reduced memory footprint and of a faster flushing process. | ||||||
|
||||||
Read-only entities are skipped by the dirty checking mechanism as illustrated by the following example: | ||||||
|
@@ -476,14 +478,28 @@ In this example, no SQL `UPDATE` was executed. | |||||
The method `Query#setReadOnly()` is an alternative to using a Jakarta Persistence query hint: | ||||||
|
||||||
[[hql-read-only-entities-native-example]] | ||||||
.Read-only entities native query example | ||||||
.Read-only entities - native - query example | ||||||
==== | ||||||
[source, java, indent=0] | ||||||
---- | ||||||
include::{example-dir-query}/HQLTest.java[tags=hql-read-only-entities-native-example] | ||||||
---- | ||||||
==== | ||||||
|
||||||
[WARNING] | ||||||
==== | ||||||
When a parent entity is in a **read-only state**, operations are still cascaded to its child associations. For example, adding a new child to a collection on a read-only parent will result in the child entity being persisted. If this behavior is not desired, the parent entity needs to be evicted from the session before making modifications. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
==== | ||||||
|
||||||
[[hql-read-only-entities-native-cascade-to-child-example]] | ||||||
.Read-only entities - native, cascade to child - query example | ||||||
==== | ||||||
[source, java, indent=0] | ||||||
---- | ||||||
include::{example-dir-query}/HQLTest.java[tags=hql-read-only-entities-native-cascade-to-child-example] | ||||||
---- | ||||||
==== | ||||||
|
||||||
[[hql-api-incremental]] | ||||||
=== Scrolling and streaming results | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -419,6 +419,10 @@ public interface Session extends SharedSessionContract, EntityManager { | |
* Change the default for entities and proxies loaded into this session | ||
* from modifiable to read-only mode, or from modifiable to read-only mode. | ||
* <p> | ||
* In some ways, Hibernate treats read-only entities the same as | ||
* entities that are not read-only; for example, it cascades | ||
* operations to associations as defined in the entity mapping. | ||
* <p> | ||
Comment on lines
+422
to
+425
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I strongly disagree with this misleading language. Please remove it. Cascading operations have absolutely nothing to do with read-onliness of an entity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see what you mean: when the parent has a child association, which uses Personally, I strongly disagree with that perspective, because even though it is technically correct in some sense, I am not aware of any special definition of a "field" that excludes the Note that this is not the scenario where you explicitly persist the child. You only persist (via loading) the parent, which causes a side-effect of persistence even though you never "authorized" the persistence of the child itself. I agree though that it should be re-phrased to include your definition that is more correct. My intention is to be explicit about covering this "severe risk" in the JavaDoc, and for that it is vital to explicitly inform the user of the practical side effect in the form of "persisting any added child association", which I also believe to be the most likely scenario to happen, as this is really close to the most trivial data model example. For example, we could be more explicit about the definition by saying "This only changes all owned fields of an entity to be read-only. Any fields not owned by the entity, such as child associations, will not be considered read-only." Can you help me come up with a reasonable comment here, considering this is the most frequent and important place any first-time users of this function will likely see? Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Well, in this case at least, that's actually quite wrong. There is indeed a
This Now, on the other hand, if this were an owned collection, you would have a strong point. I believe that the situation today is that read-only mode doesn't affect collections at all. And I think that's simply wrong. It should affect owned collections. But, again, this isn't really a problem with the documentation, it's a problem with the actual functionality. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but what I'm saying is that the Read-Only's docs don't say
In any case, as you explained the process very well, this JavaDoc should be very explicit about warning the user that cascading collections will still persist the entity. Also, I will re-affirm my stance: if you read the Hibernate 3.5 readonly docs, you will find the sentence I added to this javadoc verbatim. It does in some ways "treat read-only entities the same as entities that are not read-only". It is literally about treating an entity as a whole this way. (And I can't imagine any JavaDoc reader would gain this understanding up until now.) Thanks for reviewing the docs! |
||
* Read-only entities are not dirty-checked and snapshots of persistent | ||
* state are not maintained. Read-only entities can be modified, but | ||
* changes are not persisted. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -387,6 +387,10 @@ default Stream<R> stream() { | |
* and proxies that are loaded into the session, use | ||
* {@link Session#setDefaultReadOnly(boolean)}. | ||
* <p> | ||
* In some ways, Hibernate treats read-only entities the same as | ||
* entities that are not read-only; for example, it cascades | ||
* operations to associations as defined in the entity mapping. | ||
* <p> | ||
Comment on lines
+390
to
+393
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above; I'm hoping an expert in Hibernate terminology can fine-tune this critical point of visibility. This was the first place we analyzed when we observed this issue in a production code... |
||
* Read-only entities are not dirty-checked and snapshots of persistent | ||
* state are not maintained. Read-only entities can be modified, but | ||
* changes are not persisted. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, you suggest changing it to a note here, but keeping it as a warning below? Also, maybe you could provide some reasoning for future maintainers why this is not semantically more of a warning of a non-intuitive behavior with potentially dangerous default usage. In any case, I agree that it should probably be just a note here, while the official definition with any warnings should be located in a document dedicated to "read-onliness", similar to Hibernate 3's docs. Thanks!