Your explanation is very accurate and now I understand.
One single point: why in my case is not necessary commit and set is enough to save the changes in database? It’s about a browser screen not a editor screen in read-only mode…
Please send me a small concise example. For me it would be much easier to explain the case using your code.
But I already did (setStatus is enough to save the changes in the database):
public class TransactionBrowse extends StandardLookup<Transaction> {
@Subscribe("setBtn")
public void onSetBtnClick(Button.ClickEvent event) {
Set<Transaction> transactions = transactionsTable.getSelected();
for (Transaction a : transactions) {
if (logic) {
a.setStatus(TrStatusEnum.INVOICED);
}
}
}
}
I mean that I need a minimal CUBA project that shows the case. I don’t need all your code, just create another application that shows this exact case.
BTW, you can use zipProject task to create an archive of the project sources.
I think is pretty obvious. When I click a button I need to change some fields (attributes) according with another attributes presence/absence. This code works perfectly and I wasted my time trying to commit (as well)…
Additional commit actually perform a random saving behavior with errors.
Actually I have several if(s) and several set()s…commit()s.
if (logic1) {
a.setStatus(TrStatusEnum.INVOICED);
//dataManager.commit(a);
}
if (logic2) {
a.setStatus(TrStatusEnum.NONE);
//dataManager.commit(a);
}
Hi,
I tried to reproduce your example, see the application attached (89.2 KB) Thanks to your proper questions, we’ve found an issue in the framework and we’ll be fixing it soon.
As for the application. I’ve created a similar code and we definitely need to commit data in the screen after updating it programmatically. I just don’t understand how you get your data committed automatically, that’s why I asked you or the code.
In my case, the code
@Subscribe("transactionsTable.process")
public void onTransactionsTableProcess(Action.ActionPerformedEvent event) {
Set<Transaction> transactions = transactionsTable.getSelected();
transactions.forEach(t -> {
if (t.getStatus() == TrStatusEnum.PROCESSING) {
t.setStatus(TrStatusEnum.INVOICED);
}
});
dataContext.commit();
}
requires final commit. Without it, data won’t be updated in the database, only in the screen. You can try it by yourself, commenting out the dataContext.commit(); line, updating data and then refreshing it.
Mind the bug while experimenting, do not forget refreshing data before processing transactions.
Hello Andrey,
Thank you for your example.
Finally I found out where the difference comes from!
In my Transaction Browser screen I want also to be able to modify a Date:
<column id="myDate" editable="true" />
and I did it in this (wrong!?) way:
@Subscribe
public void onInit(InitEvent event) {
transactionsDc.addItemPropertyChangeListener(e -> {
Transaction tr = e.getItem();
dataManager.commit(tr);
});
When I press the “Process” button addItemPropertyChangeListener is triggered (I’m not sure why) and commit the (whole) transaction (the status changes included).
If there are many logics in the same screen It’s quite difficult to manage the commit()s. Maybe you suggest a guideline.
Related to the bug, good to know…however maybe explain a little bit relation between (Cuba) data container and (Spring) data context.
Best Regards,
-n
Hi,
That’s why I asked you for an example 
You did it right, using data manager to commit entities is a perfectly legal approach. But you trigger the property change listener on any property change, that’s why it runs during transaction status update. In the listener, you can use a condition like this: if ("date".equals(e.getProperty())) {/*commit data*/} to run the code only on a particular property change.
There is no spring data context in CUBA screen tough, only CUBA data context. You can read more about screen data components here. I thought that our documentation explain relations pretty clear.
And again, think about logic separation to avoid issues with transactions. It is recommended screens to be responsible for display and manual data editing only. It might be better to extract all programmatic data processing (a.k.a. business logic) to services in the core module.
Hi,
Three cheers for you, things are clearer now.
One more question regarding business separation please. Usually for this I use 1. Services (from Middleware) that can be injected in the screens controllers (through their interface).
In this respect I didn’t understand very well what Beans (@Component) are for? Seems that can help to decouple further the business logic and 2. inject their interface (or @TransactionalEventListener) into the Service(s). However Beans seems can be used also in 3. the Screens (package) and inject directly in the screens controller.
What is the best (approach) from these 3 scenarios?
Best Regards,
-n
Hi,
Think of Beans as of Spring managed utility classes that run only in one module (core or web) in contrast to Services that provide an API interface that can be used in both modules.
So, if you declare a Bean in the core module, it won’t be available in web and vice versa.
Hi,
Ok.Good.
Back to decouple logic via Service I think something is missing.
Service(StatusService.NAME)
public class StatusServiceBean implements StatusService {
public void updateTransactions(Set<Transaction> transactions){
transactions.forEach(a -> {
if (t.getStatus() == TrStatusEnum.PROCESSING) {
t.setStatus(TrStatusEnum.INVOICED);
}
});
and in Controller
@Subscribe("transactionsTable.process")
public void onTransactionsTableProcess(Action.ActionPerformedEvent event) {
Set<Transaction> transactions = transactionsTable.getSelected();
statusService.updateTransactions(transactions);
transactionsDl.load();
}
do nothing…I think the code from ServiceBean should be inside of a Transaction…
Yes, you’re right.
Remember that you pass detached entities to the service. It means that you need to attach them to Persistence Context using entity manager. To save network traffic, you can even send transaction IDs to the service instead of whole transactions and fetch them in the service. Do not forget to mark the method with @Transactional annotation, or open a transaction manually as described in the documentation.
Another option - you can use DataManager in your service, it can work with detached entities. You can save all transactions using dataManager.commit() and pass all transactions to this method.
I made these modifications and it works!
However I’m not sure that is the right approach, I have to merge any if entity t (and I have a dozen of if(s)…is there any way to gather and commit all setStatus modifications once (//em.merge(transactions)) like dataContext.commit();? I saw that dataContext is not available in the middleware.
Service(StatusService.NAME)
public class StatusServiceBean implements StatusService {
@Transactional
public void updateTransactions(Set<Transaction> transactions){
EntityManager em = persistence.getEntityManager();
transactions.forEach(a -> {
if (t.getStatus() == TrStatusEnum.PROCESSING) {
t.setStatus(TrStatusEnum.INVOICED);
em.merge(a);
}
if(logic2){
t.setStatus(TrStatusEnum.INVOICED);
em.merge(a);
}
......................
});
I don’t know how to do it…
You can commit all entities at once using data manager, as I mentioned before. Does it work for you?
I saw but I don’t know exactly where should I use it.
Service(StatusService.NAME)
public class StatusServiceBean implements StatusService {
@Transactional
public void updateTransactions(Set<Transaction> transactions){
EntityManager em = persistence.getEntityManager();
transactions.forEach(a -> {
if (t.getStatus() == TrStatusEnum.PROCESSING) {
t.setStatus(TrStatusEnum.INVOICED);
em.merge(a);
}
if(logic2){
t.setStatus(TrStatusEnum.INVOICED);
em.merge(a);
}
......................
});
dataManager.commit();
}
doesn’t work.
Well, just have a look at the DataManager’s API:
/**
* Commits new or detached entity instances to the data store.
* @param entities entities to commit
* @return set of committed instances
*/
EntitySet commit(Entity... entities);
Moreover, Datamanager works with detached entities, as I mentioned before, there is no need to merge them to the persistence context.
Sorry, I forgot to comment merge() when I posted the snippet code.
Finally done!
CommitContext commitContext = new CommitContext(transactions);
dataManager.commit(commitContext);
But I don’t understand why is necessary a new CommitContext Object instead of dataManager.commit(transactions);`
This CommitContext class is specific Cuba…such kind of persistence context…
CommitContext is just a container for entities used by DataManager. It should work fine without it, because if you look into the source code and try to debug, you’ll see that array of entities is wrapped into commit context anyway inside the data manager implementation.
For me the following code with Stream API works. It commits entities one by one though. But it’s not about CUBA, it’s about “just coding”, I can refactor it to bulk commit if needed.
@Subscribe("transactionsTable.process")
public void onTransactionsTableProcess(Action.ActionPerformedEvent event) {
Set<Transaction> transactions = transactionsTable.getSelected();
transactionsService.updateTransactions(transactions).forEach(t ->
{
Transaction tr = dataContext.merge(t);//merging an updated entity into screen data context to be able to update it programmatically
transactionsDc.replaceItem(tr);//replacing the item in the container to display it on the screen
});
}
```java
public List<Transaction> updateTransactions(Collection<Transaction> transactions) {
return transactions.stream()
.filter(t -> t.getStatus() == TrStatusEnum.PROCESSING)
.map(t -> {
t.setStatus(TrStatusEnum.INVOICED);
return dataManager.commit(t);
}).collect(Collectors.toList());
}
1 Like
Yes you are right. It’s possible to refactor in this way.
Thank you very much Andrey. I truly appreciate your professionalism!
Best Regards,
-n
You’re welcome. Enjoy CUBA! 