Author: ronvoe122
Date: 2007-12-05 08:50:40-0800
New Revision: 10489
Modified:
trunk/src/java/org/tigris/scarab/om/Issue.java
trunk/src/java/org/tigris/scarab/om/IssueManager.java
Log:
Bugfix: removed some side effects in the chaching of AttributeValues which resulted in a NPE in LuceneSearchIndex.index().
There are still side effects - this whole functionality is a mess and it will take some effort to clean it up.
Modified: trunk/src/java/org/tigris/scarab/om/Issue.java
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/java/org/tigris/scarab/om/Issue.java?view=diff&rev=10489&p1=trunk/src/java/org/tigris/scarab/om/Issue.java&p2=trunk/src/java/org/tigris/scarab/om/Issue.java&r1=10488&r2=10489
==============================================================================
--- trunk/src/java/org/tigris/scarab/om/Issue.java (original)
+++ trunk/src/java/org/tigris/scarab/om/Issue.java 2007-12-05 08:50:40-0800
@@ -846,8 +846,6 @@
Attribute attr = (Attribute)attributes.get(i);
AttributeValue aval = AttributeValue.getNewInstance(attr, this);
addAttributeValue(aval);
- String avalKey = aval.getAttribute().getName().toUpperCase();
- siaValuesMap.put(avalKey, aval);
result.put(key, aval);
}
}
@@ -1041,7 +1039,7 @@
public Map getAttributeValuesMap() throws TorqueException
{
Map result = null;
- Object obj = ScarabCache.get(this, GET_ATTRIBUTE_VALUES_MAP);
+ Object obj = getCachedObject(GET_ATTRIBUTE_VALUES_MAP);
if (obj == null)
{
final Criteria crit = new Criteria(2)
@@ -1054,7 +1052,7 @@
result.put(att.getAttribute().getName().toUpperCase(), att);
}
- ScarabCache.put(result, this, GET_ATTRIBUTE_VALUES_MAP);
+ putCachedObject(result, GET_ATTRIBUTE_VALUES_MAP);
}
else
{
@@ -3005,21 +3003,7 @@
*/
private Object getCachedObject(String methodName)
{
- Object obj = null;
- // Cache Note:
- // we check for issue id, so that we only (JCS) cache for saved issues
- // if we decide to cache results for new issues we should replace
- // this conditional with (this instanceof IssueSearch) because
- // we definitely do not want to cache those.
- if (getIssueId() == null)
- {
- obj = ScarabCache.get(this, methodName);
- }
- else
- {
- obj = getMethodResult().get(this, methodName);
- }
- return obj;
+ return getMethodResult().get(this, methodName);
}
/**
@@ -3029,15 +3013,8 @@
*/
private void putCachedObject(Object obj, String methodName)
{
- // see Cache Note above
- if (getIssueId() == null)
- {
- ScarabCache.put(obj, this, methodName);
- }
- else
- {
+ if (getIssueId()!=null)
getMethodResult().put(obj, this, methodName);
- }
}
/**
@@ -3047,21 +3024,7 @@
*/
private Object getCachedObject(String methodName, Serializable arg1)
{
- Object obj = null;
- // Cache Note:
- // we check for issue id, so that we only (JCS) cache for saved issues
- // if we decide to cache results for new issues we should replace
- // this conditional with (this instanceof IssueSearch) because
- // we definitely do not want to cache those.
- if (getIssueId() == null)
- {
- obj = ScarabCache.get(this, methodName, arg1);
- }
- else
- {
- obj = getMethodResult().get(this, methodName, arg1);
- }
- return obj;
+ return getMethodResult().get(this, methodName, arg1);
}
/**
@@ -3072,15 +3035,8 @@
private void putCachedObject(Object obj, String methodName,
Serializable arg1)
{
- // see Cache Note above
- if (getIssueId() == null)
- {
- ScarabCache.put(obj, this, methodName, arg1);
- }
- else
- {
+ if (getIssueId()!=null)
getMethodResult().put(obj, this, methodName, arg1);
- }
}
/**
@@ -3091,21 +3047,7 @@
private Object getCachedObject(String methodName,
Serializable arg1, Serializable arg2)
{
- Object obj = null;
- // Cache Note:
- // we check for issue id, so that we only (JCS) cache for saved issues
- // if we decide to cache results for new issues we should replace
- // this conditional with (this instanceof IssueSearch) because
- // we definitely do not want to cache those.
- if (getIssueId() == null)
- {
- obj = ScarabCache.get(this, methodName, arg1, arg2);
- }
- else
- {
- obj = getMethodResult().get(this, methodName, arg1, arg2);
- }
- return obj;
+ return getMethodResult().get(this, methodName, arg1, arg2);
}
/**
@@ -3116,15 +3058,8 @@
private void putCachedObject(Object obj, String methodName,
Serializable arg1, Serializable arg2)
{
- // see Cache Note above
- if (getIssueId() == null)
- {
- ScarabCache.put(obj, this, methodName, arg1, arg2);
- }
- else
- {
+ if (getIssueId() != null)
getMethodResult().put(obj, this, methodName, arg1, arg2);
- }
}
@@ -3564,11 +3499,10 @@
}
activitySet = attachActivitySet(activitySet, user, attachment );
- final LinkedMap avMap = getModuleAttributeValuesMap();
+ final LinkedMap avMap = getModuleAttributeValuesMap(true);
AttributeValue oldAttVal = null;
AttributeValue newAttVal = null;
final Iterator iter = newAttVals.keySet().iterator();
- boolean attValDeleted = false;
while (iter.hasNext())
{
final Integer attrId = (Integer)iter.next();
@@ -3582,11 +3516,6 @@
|| newAttValValue == null)
)
{
- if (Log.get().isDebugEnabled())
- {
- Log.get().debug("Attribute: " + attr.getName() +
- " has newAttValValue = " + newAttValValue);
- }
if (newAttValValue != null && newAttValValue.length() > 0)
{
oldAttVal.setProperties(newAttVal);
@@ -3594,21 +3523,11 @@
else
{
oldAttVal.setDeleted(true);
- Log.get().debug("setDeleted(true)");
- attValDeleted = true;
}
oldAttVal.startActivitySet(activitySet);
oldAttVal.save();
}
-
}
- if (attValDeleted)
- {
- //Remove attribute value map from cache
- getMethodResult().remove(this, GET_MODULE_ATTRVALUES_MAP,
- Boolean.TRUE);
- }
-
index();
return activitySet;
@@ -3922,18 +3841,19 @@
public String toString()
{
- String id = null;
- try
- {
- id = isNew() ? "New issue" : getUniqueId();
- }
- catch (Exception e)
- {
- id = "Error in getting unique id";
- Log.get().warn(id, e);
- }
+ // Only return the issueId
+ // because toString() is used as the groupKey
+ // in the methodResultCache.
+ // The result of BaseIssue.toString()
+ // changes if the issue is changed.
+ // Thats why the methodResultCache would be poluted by
+ // multiple entries for the same method call and the same issue
+ // if an issue was changed.
+ // This is a flaw in the design of the Torque-methodCache.
+ // The groupKey should not be constructed by toString.
+ // Better would be, to use hashValue().
- return super.toString() + '{' + id + '}';
+ return getIssueId()==null ? "new" : getIssueId().toString();
}
/**
Modified: trunk/src/java/org/tigris/scarab/om/IssueManager.java
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/java/org/tigris/scarab/om/IssueManager.java?view=diff&rev=10489&p1=trunk/src/java/org/tigris/scarab/om/IssueManager.java&p2=trunk/src/java/org/tigris/scarab/om/IssueManager.java&r1=10488&r2=10489
==============================================================================
--- trunk/src/java/org/tigris/scarab/om/IssueManager.java (original)
+++ trunk/src/java/org/tigris/scarab/om/IssueManager.java 2007-12-05 08:50:40-0800
@@ -225,8 +225,9 @@
Serializable obj = getInstance(key);
if (obj != null)
{
- getMethodResult().remove(obj, Issue.GET_MODULE_ATTRVALUES_MAP);
- getMethodResult().remove(obj, Issue.GET_USER_ATTRIBUTEVALUES);
+ getMethodResult().removeAll(obj, Issue.GET_MODULE_ATTRVALUES_MAP);
+ getMethodResult().removeAll(obj, Issue.GET_USER_ATTRIBUTEVALUES);
+ getMethodResult().removeAll(obj, Issue.GET_ATTRIBUTE_VALUES_MAP);
}
}
catch(TorqueException e)
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.