Hi Dave,
Today Mitesh and I resolved the open TODO items in the JPA Datamapper implementation. There's one remaining question, we'd like to run by you:3. Go through TODO comments in code and resolve them
In DatamapperRefererManagerImpl: -clearReferrers() -clearReferrers(WebsiteData website) These methods clear referers who have null or empty excerpt. The hibernate impl used database specific way for finding empty excerptDatamapper impl uses db independent way by using expression "excerpt like ''"
I could verify that the approach works for mssql, oracle, derby, and mysql We removed this TODO item. Hope that's ok! We also added two Comparator classes: src/org/apache/roller/pojos/StatCountCountComparator.java src/org/apache/roller/pojos/TagStatCountComparator.java Thanks, Mitesh & Markus.
Index: sandbox/jdobackend/src/org/apache/roller/pojos/RefererData.orm.xml
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/pojos/RefererData.orm.xml (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/pojos/RefererData.orm.xml (working copy)
@@ -71,10 +71,10 @@
<named-query name="RefererData.clearDayHitsByWebsite">
<query>UPDATE RefererData r SET r.dayHits=0 where r.website = ?1</query>
</named-query>
- <named-query name="RefererData.deleteByNullOrEmptyExcerpt">
+ <named-query name="RefererData.removeByNullOrEmptyExcerpt">
<query>DELETE FROM RefererData r WHERE r.excerpt IS NULL OR r.excerpt LIKE ''</query>
</named-query>
- <named-query name="RefererData.deleteByNullOrEmptyExcerpt&Website">
+ <named-query name="RefererData.removeByNullOrEmptyExcerpt&Website">
<query>DELETE FROM RefererData r WHERE r.website = ?1 AND (r.excerpt IS NULL OR r.excerpt LIKE '')</query>
</named-query>
<attributes>
Index: sandbox/jdobackend/src/org/apache/roller/pojos/WeblogEntryTagAggregateData.orm.xml
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/pojos/WeblogEntryTagAggregateData.orm.xml (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/pojos/WeblogEntryTagAggregateData.orm.xml (working copy)
@@ -67,21 +67,17 @@
<named-query name="WeblogEntryTagAggregateData.updateAddToTotalByName&WeblogNull">
<query>UPDATE WeblogEntryTagAggregateData w SET w.total = w.total + ?1 WHERE w.name = ?2 AND w.weblog IS NULL</query>
</named-query>
- <named-query name="WeblogEntryTagAggregateData.updateTotalByName&WeblogNull">
+ <named-query name="WeblogEntryTagAggregateData.updateMinusFromTotalByName&WeblogNull">
<query>UPDATE WeblogEntryTagAggregateData w SET w.total = w.total - ?1 WHERE w.name = ?2 AND w.weblog IS NULL</query>
</named-query>
<named-query name="WeblogEntryTagAggregateData.removeByTotalLessEqual">
<query>DELETE FROM WeblogEntryTagAggregateData w WHERE w.total <= ?1</query>
</named-query>
- <named-query name="WeblogEntryTagAggregateData.deleteByWeblog">
+ <named-query name="WeblogEntryTagAggregateData.removeByWeblog">
<query>DELETE FROM WeblogEntryTagAggregateData w WHERE w.weblog = ?1</query>
</named-query>
- <named-query name="WeblogEntryTagAggregateData.deleteByTotalLEZero">
- <query>DELETE FROM WeblogEntryTagAggregateData w WHERE w.total <= 0</query>
- </named-query>
-
<attributes>
<id name="id">
<column name="id"/>
Index: sandbox/jdobackend/src/org/apache/roller/business/jpa/JPAUserManagerImpl.java
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/business/jpa/JPAUserManagerImpl.java (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/business/jpa/JPAUserManagerImpl.java (working copy)
@@ -41,7 +41,7 @@
for(Iterator iter = tags.iterator(); iter.hasNext();) {
TagStat stat = (TagStat) iter.next();
JPAUpdateQuery query = ((JPAPersistenceStrategy)strategy).newUpdateQuery(
- "WeblogEntryTagAggregateData.updateTotalByName&WeblogNull");
+ "WeblogEntryTagAggregateData.updateMinusFromTotalByName&WeblogNull");
query.updateAll(
new Object[] {Integer.valueOf(stat.getCount()),
stat.getName() });
Index: sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperRefererManagerImpl.java
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperRefererManagerImpl.java (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperRefererManagerImpl.java (working copy)
@@ -24,13 +24,13 @@
import java.util.Calendar;
import java.util.Date;
import java.util.Collections;
+import java.util.Comparator;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.commons.lang.StringUtils;
import org.apache.roller.RollerException;
-
import org.apache.roller.business.Roller;
import org.apache.roller.business.RollerFactory;
import org.apache.roller.business.UserManager;
@@ -41,6 +41,7 @@
import org.apache.roller.pojos.StatCount;
import org.apache.roller.pojos.WeblogEntryData;
import org.apache.roller.pojos.WebsiteData;
+import org.apache.roller.pojos.StatCountCountComparator;
import org.apache.roller.util.LinkbackExtractor;
import org.apache.roller.util.Utilities;
@@ -57,6 +58,10 @@
protected static final String DAYHITS = "dayHits";
protected static final String TOTALHITS = "totalHits";
+
+ private static final Comparator statCountCountReverseComparator =
+ Collections.reverseOrder(StatCountCountComparator.getInstance());
+
/** The strategy for this manager. */
protected DatamapperPersistenceStrategy strategy;
@@ -80,14 +85,8 @@
*/
public void clearReferrers() throws RollerException {
clearDayHits();
-
- //TODO: Datamapper port: The original code used two different expressions
- // to find empty exerpt
- // (1) excerpt like '' for mssql, oracle and derby
- // (2) excerpt = '' for all other
- // I could verify that (1) works for mssql, oralce, derby, and mysql
strategy.newRemoveQuery(RefererData.class,
- "RefererData.deleteByNullOrEmptyExcerpt").removeAll();
+ "RefererData.removeByNullOrEmptyExcerpt").removeAll();
}
abstract protected void clearDayHits() throws RollerException;
@@ -98,9 +97,8 @@
public void clearReferrers(WebsiteData website) throws RollerException {
clearDayHitsByWebsite(website);
- //TODO: Datamapper port: Same comments as for method clearReferrers() apply for the expression
strategy.newRemoveQuery(RefererData.class,
- "RefererData.deleteByNullOrEmptyExcerpt&Website").removeAll(website);
+ "RefererData.removeByNullOrEmptyExcerpt&Website").removeAll(website);
}
@@ -129,8 +127,6 @@
*/
public void applyRefererFilters(WebsiteData website)
throws RollerException {
- // TODO not implemented
-
if (null == website) throw new RollerException("website is null");
if (null == website.getBlacklist()) return;
@@ -207,9 +203,10 @@
"statCount.weblogDayHits",
hits.longValue()));
}
- //TODO Uncomment following once integrated with code
- //Collections.sort(results, StatCount.getComparator());
- Collections.reverse(results);
+ // Original query ordered by desc hits.
+ // JPA QL doesn't allow queries to be ordered by agregates; do it in memory
+ Collections.sort(results, statCountCountReverseComparator);
+
return results;
}
@@ -219,8 +216,8 @@
if (log.isDebugEnabled()) {
log.debug("getHits: " + website.getName());
}
- //TODO: DatamapperPort. This original query retrieves both SUM(r.dayHits), SUM(r.totalHits)
- //The method only comsumes one of them. We can optimize the logic to retrive only the
+ //TODO: DatamapperPort. This query retrieves both SUM(r.dayHits), SUM(r.totalHits)
+ //The method only comsumes one of them. We can optimize the logic to retrieve only the
//requied SUM
DatamapperQuery query = strategy.newQuery(RefererData.class,
"RefererData.getHitsByWebsite.enabled&Website.id");
@@ -296,7 +293,10 @@
if (null == entryid)
throw new RollerException("entryid is null");
//TODO: DataMapperPort: Change calling code to pass WebLogEntryData instead of id
- return (List) strategy.newQuery(RefererData.class,
+ // we should change calling code to pass instance of WeblogEntryData instead
+ // of extracting and passing id. Once that is done, change the code below to
+ // skip the load (Please note that the load below will always find the enty in cache)
+ return (List) strategy.newQuery(RefererData.class,
"RefererData.getByWeblogEntry&TitleNotNull&ExcerptNotNullOrderByTotalHitsDesc").
execute(strategy.load(WeblogEntryData.class,entryid));
}
Index: sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperUserManagerImpl.java
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperUserManagerImpl.java (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperUserManagerImpl.java (working copy)
@@ -29,23 +29,7 @@
import org.apache.roller.business.pings.AutoPingManager;
import org.apache.roller.business.pings.PingTargetManager;
import org.apache.roller.config.RollerConfig;
-import org.apache.roller.pojos.AutoPingData;
-import org.apache.roller.pojos.BookmarkData;
-import org.apache.roller.pojos.FolderData;
-import org.apache.roller.pojos.PermissionsData;
-import org.apache.roller.pojos.PingQueueEntryData;
-import org.apache.roller.pojos.PingTargetData;
-import org.apache.roller.pojos.RefererData;
-import org.apache.roller.pojos.StatCount;
-import org.apache.roller.pojos.UserData;
-import org.apache.roller.pojos.WeblogCategoryData;
-import org.apache.roller.pojos.WeblogEntryData;
-import org.apache.roller.pojos.WeblogTemplate;
-import org.apache.roller.pojos.WebsiteData;
-import org.apache.roller.pojos.CommentData;
-import org.apache.roller.pojos.WeblogEntryTagData;
-import org.apache.roller.pojos.WeblogEntryTagAggregateData;
-import org.apache.roller.pojos.RoleData;
+import org.apache.roller.pojos.*;
import java.util.ArrayList;
import java.util.Collections;
@@ -56,6 +40,7 @@
import java.util.Map;
import java.util.TreeMap;
import java.util.Collection;
+import java.util.Comparator;
/*
* DatamapperUserManagerImpl.java
@@ -68,9 +53,11 @@
/** The logger instance for this class. */
private static Log log = LogFactory.getLog(DatamapperUserManagerImpl.class);
- protected DatamapperPersistenceStrategy strategy;
-
+ private static final Comparator statCountCountReverseComparator =
+ Collections.reverseOrder(StatCountCountComparator.getInstance());
+ protected DatamapperPersistenceStrategy strategy;
+
// cached mapping of weblogHandles -> weblogIds
private Map weblogHandleToIdMap = new Hashtable();
@@ -105,7 +92,6 @@
/**
* convenience method for removing contents of a weblog.
* TODO BACKEND: use manager methods instead of queries here
- * TODO DatamapperPort: Use bulk deletes instead of current approach
*/
private void removeWebsiteContents(WebsiteData website)
throws RollerException {
@@ -128,12 +114,12 @@
// delete all weblog tag aggregates
strategy.newRemoveQuery(
WeblogEntryTagAggregateData.class,
- "WeblogEntryTagAggregateData.deleteByWeblog").removeAll(website);
+ "WeblogEntryTagAggregateData.removeByWeblog").removeAll(website);
// delete all bad counts
strategy.newRemoveQuery(
WeblogEntryTagAggregateData.class,
- "WeblogEntryTagAggregateData.deleteByTotalLEZero").removeAll();
+ "WeblogEntryTagAggregateData.removeByTotalLessEqual").removeAll(new Integer(0));
// Remove the website's ping queue entries
@@ -200,9 +186,8 @@
this.strategy.remove(rootCat);
}
- //remove permissions
- //TODO: Datamapper: this is a workaround for toplink bug that requires
- //to clean up from non owning side for removed objects.
+ // remove permissions
+ // make sure that both sides of the relationship are maintained
for (Iterator iterator = website.getPermissions().iterator(); iterator.hasNext();) {
PermissionsData perms = (PermissionsData) iterator.next();
//Remove it from database
@@ -210,7 +195,7 @@
//Remove it from website
iterator.remove();
//Remove it from corresponding user
- UserData user = perms.getUser(); //(UserData) getManagedObject(perms.getUser());
+ UserData user = perms.getUser();
user.getPermissions().remove(perms);
}
@@ -228,8 +213,7 @@
public void removeUser(UserData user) throws RollerException {
//remove permissions
- //TODO: Datamapper: this is a workaround for toplink bug that requires
- //to clean up from non owning side for removed objects.
+ // make sure that both sides of the relationship are maintained
for (Iterator iterator = user.getPermissions().iterator(); iterator.hasNext();) {
PermissionsData perms = (PermissionsData) iterator.next();
//Remove it from database
@@ -237,7 +221,7 @@
//Remove it from website
iterator.remove();
//Remove it from corresponding user
- WebsiteData website = perms.getWebsite(); //(WebsiteData) getManagedObject(perms.getWebsite());
+ WebsiteData website = perms.getWebsite();
website.getPermissions().remove(perms);
}
@@ -1078,12 +1062,6 @@
queryResults = (List) query.execute(endDate);
}
- // TODO: DatamapperPort - The original query list column not present in group by clause in select clause
- // this is not allowed by JPA spec (and many db vendors).
- // Changed the select clause to match group by clause.
- // Currently, the only caller of this method is SiteModel.getMostCommentedWeblogs() (apart from a junit test)
- // check with roller developers what is the expected behavior of this query
-
List results = new ArrayList();
for (Iterator iter = queryResults.iterator(); iter.hasNext();) {
Object[] row = (Object[]) iter.next();
@@ -1094,9 +1072,10 @@
"statCount.weblogCommentCountType", // stat type
((Long)row[0]).longValue())); // # comments
}
- //TODO Uncomment following once integrated with code
- //Collections.sort(results, StatCount.getComparator());
- Collections.reverse(results);
+ // Original query ordered by desc # comments.
+ // JPA QL doesn't allow queries to be ordered by agregates; do it in memory
+ Collections.sort(results, statCountCountReverseComparator);
+
return results;
}
Index: sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperWeblogManagerImpl.java
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperWeblogManagerImpl.java (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperWeblogManagerImpl.java (working copy)
@@ -45,12 +45,14 @@
import org.apache.roller.pojos.StatCount;
import org.apache.roller.pojos.TagStat;
import org.apache.roller.pojos.TagStatComparator;
+import org.apache.roller.pojos.TagStatCountComparator;
import org.apache.roller.pojos.WeblogCategoryData;
import org.apache.roller.pojos.WeblogEntryData;
import org.apache.roller.pojos.WeblogEntryTagAggregateData;
import org.apache.roller.pojos.WeblogEntryTagData;
import org.apache.roller.pojos.WebsiteData;
import org.apache.roller.pojos.EntryAttributeData;
+import org.apache.roller.pojos.StatCountCountComparator;
import org.apache.roller.util.DateUtil;
/*
@@ -70,10 +72,16 @@
private Hashtable entryAnchorToIdMap = new Hashtable();
/* inline creation of reverse comparator, anonymous inner class */
- private Comparator reverseComparator = new ReverseComparator();
+ private static final Comparator reverseComparator = new ReverseComparator();
- private Comparator tagStatComparator = new TagStatComparator();
+ private static final Comparator tagStatNameComparator = new TagStatComparator();
+ private static final Comparator tagStatCountReverseComparator =
+ Collections.reverseOrder(TagStatCountComparator.getInstance());
+
+ private static final Comparator statCountCountReverseComparator =
+ Collections.reverseOrder(StatCountCountComparator.getInstance());
+
public DatamapperWeblogManagerImpl
(DatamapperPersistenceStrategy strategy) {
log.debug("Instantiating Datamapper Weblog Manager");
@@ -932,9 +940,10 @@
"statCount.weblogEntryCommentCountType", // stat desc
((Long)row[0]).longValue())); // count
}
- //TODO Uncomment following once integrated with code
- //Collections.sort(results, StatCount.getComparator());
- Collections.reverse(results);
+ // Original query ordered by desc count.
+ // JPA QL doesn't allow queries to be ordered by agregates; do it in memory
+ Collections.sort(results, statCountCountReverseComparator);
+
return results;
}
@@ -1010,8 +1019,6 @@
}
}
- // TODO queryString.append("order by sum(total) desc"); ???
-
double min = Integer.MAX_VALUE;
double max = Integer.MIN_VALUE;
@@ -1039,7 +1046,7 @@
}
// sort results by name, because query had to sort by total
- Collections.sort(results, tagStatComparator);
+ Collections.sort(results, tagStatNameComparator);
return results;
}
@@ -1114,14 +1121,15 @@
Object[] row = (Object[]) iter.next();
TagStat ce = new TagStat();
ce.setName((String) row[0]);
- //TODO: DatamapperPort. The query retrieves SUM(w.total) which is always long
- //TagStat should be changed to receive count as long
+ // The JPA query retrieves SUM(w.total) always as long
ce.setCount(((Long) row[1]).intValue());
results.add(ce);
}
- if (!sortByName) {
- Collections.sort(results, tagStatComparator);
+ if (sortByName) {
+ Collections.sort(results, tagStatNameComparator);
+ } else {
+ Collections.sort(results, tagStatCountReverseComparator);
}
return results;
Index: sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperPlanetManagerImpl.java
===================================================================
--- sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperPlanetManagerImpl.java (revision 500446)
+++ sandbox/jdobackend/src/org/apache/roller/business/datamapper/DatamapperPlanetManagerImpl.java (working copy)
@@ -271,11 +271,11 @@
if (startDate != null) {
params = new Object[] {endDate, startDate};
query = strategy.newQuery(PlanetEntryData.class,
- "PlanetEntryData.getByExternalOrInternalGroup&EndDate&StartDateOrderByPubTimeDesc");
+ "PlanetEntryData.getByExternalOrInternalGroup&EndDate&StartDateOrderByPubTimeDesc");
} else {
params = new Object[] {endDate};
query = strategy.newQuery(PlanetEntryData.class,
- "PlanetEntryData.getByExternalOrInternalGroup&EndDateOrderByPubTimeDesc");
+ "PlanetEntryData.getByExternalOrInternalGroup&EndDateOrderByPubTimeDesc");
}
// TODO handle offset and length
}
rollertodo.jar
Description: application/java-archive
