Re: svn commit: r1177789 - /ofbiz/branches/release10.04/framework/webapp/src/org/ofbiz/webapp/stats/ServerHitBin.java

[Jacques Le Roux]

Scott Gray wrote:

Two things:
1. By asking "What bug?" I was trying to remind you to add proper details to 
commit messages.

Agreed and done

2. Are you sure this is the correct fix?  I'm pretty sure we moved away from 
using a default delegator when the tenant stuff was
added.

Actually, I guess this part could be ommited (just belt and suspenders)

+        if (delegator == null){
+            delegator = DelegatorFactory.getDelegator("default");
       }

There is another use in R10.04 (EntityExpr.java[210])
// this will be the common case for now as the delegator isn't available where 
we want to do this
// we'll cheat a little here and assume the default delegator

So when doing a quick review I thought it could not hurt before throwing an exception. Maybe in tenant case it could be a problem indeed and should be rather removed (I doubt the delegator could miss in the request and in such case it's indeed maybe better to simply throw an exception)

Maybe the deeper question is "Why is delegator name empty? Why is there no delegator 
instance in the request?".  Not everything
is solvable with a quick fix, it is always better to try and understand the 
cause of the problem before coming up with a
solution.

Jacques

Note the delegator missed in the session not the request, as stated above the 2d block is just belt and suspenders, maybe useless and wrong indeed

Regards
Scott

On 1/10/2011, at 5:54 PM, Jacques Le Roux wrote:

http://markmail.org/message/4b37lgy7ymsfg43u

I was able to reproduce

Jacques

Scott Gray wrote:

What bug?

On 1/10/2011, at 8:25 AM, jler...@apache.org wrote:

Author: jleroux
Date: Fri Sep 30 19:25:16 2011
New Revision: 1177789

URL: http://svn.apache.org/viewvc?rev=1177789&view=rev
Log:
Fix a bug reported by Martin Sanchez Vivo on user ML

Modified:
  
ofbiz/branches/release10.04/framework/webapp/src/org/ofbiz/webapp/stats/ServerHitBin.java

Modified: 
ofbiz/branches/release10.04/framework/webapp/src/org/ofbiz/webapp/stats/ServerHitBin.java
URL:
http://svn.apache.org/viewvc/ofbiz/branches/release10.04/framework/webapp/src/org/ofbiz/webapp/stats/ServerHitBin.java?rev=1177789&r1=1177788&r2=1177789&view=diff
============================================================================== 
---
ofbiz/branches/release10.04/framework/webapp/src/org/ofbiz/webapp/stats/ServerHitBin.java
 (original) +++
ofbiz/branches/release10.04/framework/webapp/src/org/ofbiz/webapp/stats/ServerHitBin.java
 Fri Sep 30 19:25:16 2011 @@ -111,10
   +111,9 @@ public class ServerHitBin { }

   protected static void countHit(String baseId, int type, HttpServletRequest 
request, long startTime, long runningTime,
GenericValue userLogin, boolean isOriginal) {
-        String delegatorName = (String) 
request.getSession().getAttribute("delegatorName");
-        Delegator delegator = null;
-        if (UtilValidate.isNotEmpty(delegatorName)) {
-            delegator = DelegatorFactory.getDelegator(delegatorName);
+        Delegator delegator = (Delegator)request.getAttribute("delegator");
+        if (delegator == null){
+            delegator = DelegatorFactory.getDelegator("default");
       }
       if (delegator == null) {
           throw new IllegalArgumentException("In countHit could not find a 
delegator or delegatorName to work from");

smime.p7s
Description: S/MIME cryptographic signature