konakart application design problem

[nclemeur]

Hello all,

Today I was looking a bit more in the struts side of the konakart demo application and it seems to me that there is a major flaw in the current design. Indeed, one of the basic principle of struts is to NOT have any instance variables available in an Action (see for example http://struts.apache.org/1.x/userGuide/building_controller.html#action_design_guide for more information). In the BaseAction, there are 2 pretty important member variable (kkAppEng and custId) which violates this principles. This is really a major problem in a heavily loaded application where multiple requests originating from multiple clients would be able to "see" each kkAppEng... I could easily simulate a problem by setting a breakpoint in an execute method and then having 2 clients connecting. I could easily add an item to the shopping cart of another client.

I am going to look at solving that problem now, but I would really recommend to patch that version asap.

Cheers

Nicolas

[julie]

Hi Nicolas,

Well spotted. There is a race condition which could manifest itself in a heavily loaded system when two customers perform the same action at exactly the same time. Our actions are performed very quickly since we move most of the persistence, and "business logic" to a separate application layer (client engine) as advised in the Struts guidelines  which is why the "interference" window is so small and will very rarely cause a problem.

However, this has been fixed already and will be available in the next release coming very shortly, hopefully by the end of the week.

Regards,

Julie

[nclemeur]

I am not sure I really agree with everything. Moving things into another layer does not make it faster. Also, the problem would definitely be worse on a heavily loaded system, but it can happen with only 2 clients connected as well... It's just a matter of bad timing of requests.

I still think that would require a patch to be supplied as not everyone would have the possibility to upgrade very easily. Specially, since it's involves the struts part of the application which is quite likely to be customized. At a very minimum I would think that a "known issues" page would be necessary and a quick explanation on how to solve it would be nice. I'll try to post my solution later on.

I hope I am not coming accross as someone difficult, but I really hoping to improve konakart this way. I guess this would be better suited in an issue tracking system.


Cheers

Nicolas

[julie]

I am not sure I really agree with everything. Moving things into another layer does not make it faster. Also, the problem would definitely be worse on a heavily loaded system, but it can happen with only 2 clients connected as well... It's just a matter of bad timing of requests.

What I meant is that those variables are calculated normally at the start of each action and then passed on to an engine call. If they change while the engine call is working we don't care. Therefore the time window where interference could take place is reduced.

I still think that would require a patch to be supplied as not everyone would have the possibility to upgrade very easily. Specially, since it's involves the struts part of the application which is quite likely to be customized. At a very minimum I would think that a "known issues" page would be necessary and a quick explanation on how to solve it would be nice. I'll try to post my solution later on.

The way we solved it is to pass kkAppEng and custId as parameters rather than having them as instance variables. For users that have customized the struts actions the changes to make are relatively simple. However the plan is certainly to document them. If someone doesn't want to upgrade he can take the new Struts actions and use them in his current version or make the same changes that we made to his current actions.

I hope I am not coming accross as someone difficult, but I really hoping to improve konakart this way. I guess this would be better suited in an issue tracking system.

Not at all. We appreciate your help and suggestions, and realise that fixing bugs, does make KonaKart a better product 


Regards,

Julie