danhaywood commented on code in PR #2412:
URL: https://github.com/apache/causeway/pull/2412#discussion_r1605804219
##########
api/applib/src/main/java/org/apache/causeway/applib/services/publishing/spi/EntityPropertyChangeSubscriber.java:
##########
@@ -48,4 +49,5 @@ public interface EntityPropertyChangeSubscriber extends
HasEnabling {
*/
void onChanging(EntityPropertyChange entityPropertyChange);
+ void onBulkChanging(Can<EntityPropertyChange> entityPropertyChanges);
Review Comment:
should be `default` for backward compatibliity
##########
extensions/security/audittrail/applib/src/main/java/org/apache/causeway/extensions/audittrail/applib/dom/AuditTrailEntryRepository.java:
##########
@@ -38,6 +39,8 @@ public interface AuditTrailEntryRepository {
AuditTrailEntry createFor(final EntityPropertyChange change);
+ Can<AuditTrailEntry> createForBulk(final Can<EntityPropertyChange>
entityPropertyChanges);
Review Comment:
should be `default` for backward compatibliity
##########
core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/publish/EntityPropertyChangePublisherDefault.java:
##########
@@ -66,6 +67,9 @@ public class EntityPropertyChangePublisherDefault implements
EntityPropertyChang
private Can<EntityPropertyChangeSubscriber> enabledSubscribers =
Can.empty();
+ @Value("${entity.property.change.publisher.bulk.threshold:1}")
Review Comment:
This property name needs to change, and also should be read from
`CausewayConfiguration` so that it is typesafe.
The property name should be
`causeway.core.runtime-services.entity-property-change-publisher.bulk.threshold`
##########
api/applib/src/main/java/org/apache/causeway/applib/services/repository/RepositoryService.java:
##########
@@ -84,6 +84,14 @@ public interface RepositoryService {
*/
<T> T detachedEntity(@NonNull T entity);
+ /**
+ * Suspends flushing transaction for {@param aClass} instances until the
{@param bulkMode} is turned off again.
+ * <p>
+ * Usage should be wrapped in a try {} finally {} construction.
+ *
+ */
+ <T extends Class> void setBulkMode(final T aClass, final Boolean bulkMode);
Review Comment:
Don't like this design, it runs the risk of leaving the service in the
'bulk' mode. Also, while the impl is on a thread-local, the API suggests that
the service is stateful, which is a no-no.
Instead, provide an API that accepts a lambda and does all the work within,
eg:
`execInBulk( Callable )`
##########
persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/repository/RepositoryServiceDefault.java:
##########
@@ -116,14 +119,24 @@ public <T> T persist(final T domainObject) {
return domainObject;
}
-
@Override
public <T> T persistAndFlush(final T object) {
persist(object);
- transactionService.flushTransaction();
+ if(!threadLocal.get().getOrDefault(object.getClass(), Boolean.FALSE)) {
+ transactionService.flushTransaction();
+ }
return object;
}
+ @Override
+ public <T extends Class> void setBulkMode(final T aClass, final Boolean
bulkMode) {
Review Comment:
See previous comment, instead of this setter we use an
`execInBulk(Callable)` action.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@causeway.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
