Merge pull request #361 from zanata/rhbz1060598

rhbz1056849 - Fix wrong stats calculation.

zanata-war/src/main/java/org/zanata/dao/LocaleDAO.java

@@ -69,4 +69,8 @@ public List<HLocale> findAllActiveAndEnabledByDefault() {
         return findByCriteria(Restrictions.eq("active", true),
                 Restrictions.eq("enabledByDefault", true));
     }
+
+    public List<HLocale> findAll() {
+        return findByCriteria(); // Return all of them
+    }
 }

zanata-war/src/main/java/org/zanata/service/VersionStateCache.java

@@ -42,4 +42,10 @@ public interface VersionStateCache {
 
     WordStatistic getVersionStatistics(Long projectIterationId,
             LocaleId localeId);
+
+    /**
+     * Clears all caches for a single project version (all locales).
+     * @param versionId All cached stats for this version will be cleared.
+     */
+    void clearVersionStatsCache(Long versionId);
 }

zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java

@@ -47,6 +47,7 @@
 import org.zanata.dao.DocumentDAO;
 import org.zanata.dao.ProjectDAO;
 import org.zanata.dao.TextFlowTargetDAO;
+import org.zanata.events.TextFlowTargetStateEvent;
 import org.zanata.model.HCopyTransOptions;
 import org.zanata.model.HDocument;
 import org.zanata.model.HLocale;
@@ -58,6 +59,7 @@
 import org.zanata.service.CopyTransService;
 import org.zanata.service.LocaleService;
 import org.zanata.service.ValidationService;
+import org.zanata.service.VersionStateCache;
 import org.zanata.webtrans.shared.model.ValidationAction;
 
 import com.google.common.base.Optional;
@@ -69,9 +71,6 @@
 @Scope(ScopeType.STATELESS)
 @Slf4j
 public class CopyTransServiceImpl implements CopyTransService {
-    @In
-    private EntityManager entityManager;
-
     @In
     private LocaleService localeServiceImpl;
 
@@ -87,6 +86,9 @@ public class CopyTransServiceImpl implements CopyTransService {
     @In
     private ValidationService validationServiceImpl;
 
+    @In
+    private VersionStateCache versionStateCacheImpl;
+
     @Observer(TranslatedDocResourceService.EVENT_COPY_TRANS)
     public void runCopyTrans(Long docId, String project, String iterationSlug) {
         HDocument document = documentDAO.findById(docId, true);
@@ -250,6 +252,7 @@ private void saveCopyTransMatch(HTextFlowTarget matchingTarget,
 
         HTextFlowTarget hTarget =
             textFlowTargetDAO.getOrCreateTarget(originalTf, matchingTarget.getLocale());
+        ContentState prevState = hTarget.getId() == null ? New : hTarget.getState();
         if (shouldOverwrite(hTarget, copyState)) {
             // NB we don't touch creationDate
             hTarget.setTextFlowRevision(originalTf.getRevision());
@@ -270,6 +273,10 @@ private void saveCopyTransMatch(HTextFlowTarget matchingTarget,
                 hTarget.setComment(hcomment);
             }
             hcomment.setComment(createComment(matchingTarget));
+
+            // TODO Maybe we should think about registering a Hibernate
+            // integrator for these updates
+            signalCopiedTranslation(hTarget, prevState);
         }
     }
 
@@ -508,6 +515,23 @@ private boolean validationTranslations(ContentState newState,
         return false;
     }
 
+    private void signalCopiedTranslation(HTextFlowTarget target,
+            ContentState previousState) {
+        /* Using a direct method call instead of an event because it's easier to
+         * read. Since these events are being called synchronously (as opposed
+         * to an 'after Transaction' events), there is no big performance gain
+         * and makes the code easier to read and navigate.
+         */
+        HDocument document = target.getTextFlow()
+                .getDocument();
+        TextFlowTargetStateEvent updateEvent =
+                new TextFlowTargetStateEvent(null, document
+                        .getProjectIteration().getId(), document.getId(), target
+                        .getTextFlow().getId(), target.getLocaleId(),
+                        target.getId(), target.getState(), previousState);
+        versionStateCacheImpl.textFlowStateUpdated(updateEvent);
+    }
+
     /**
      * Holds the result of a match evaluation in the form of a boolean, and the
      * corresponding action to be taken for the result.

zanata-war/src/main/java/org/zanata/service/impl/DocumentServiceImpl.java

@@ -45,6 +45,7 @@
 import org.zanata.service.DocumentService;
 import org.zanata.service.LocaleService;
 import org.zanata.service.LockManagerService;
+import org.zanata.service.VersionStateCache;
 
 /**
  * Default implementation of the {@link DocumentService} business service
@@ -74,6 +75,9 @@ public class DocumentServiceImpl implements DocumentService {
     @In
     private LockManagerService lockManagerServiceImpl;
 
+    @In
+    private VersionStateCache versionStateCacheImpl;
+
     @In
     private ResourceUtils resourceUtils;
 
@@ -154,11 +158,14 @@ public HDocument saveDocument(String projectSlug, String iterationSlug,
         documentDAO.flush();
 
         long actorId = authenticatedAccount.getPerson().getId();
-        if (changed && Events.exists()) {
-            Events.instance().raiseTransactionSuccessEvent(
-                    DocumentUploadedEvent.EVENT_NAME,
-                    new DocumentUploadedEvent(actorId, document.getId(), true, hLocale
-                            .getLocaleId()));
+        if (changed) {
+            if( Events.exists() ) {
+                Events.instance().raiseTransactionSuccessEvent(
+                        DocumentUploadedEvent.EVENT_NAME,
+                        new DocumentUploadedEvent(actorId, document.getId(),
+                                true, hLocale.getLocaleId()));
+            }
+            clearStatsCacheForUpdatedDocument(document);
         }
 
         if (copyTrans && nextDocRev == 1) {
@@ -176,6 +183,7 @@ public void makeObsolete(HDocument document) {
         document.setObsolete(true);
         documentDAO.makePersistent(document);
         documentDAO.flush();
+        clearStatsCacheForUpdatedDocument(document);
     }
 
     /**
@@ -189,4 +197,9 @@ private void copyTranslations(HDocument document) {
             copyTransServiceImpl.copyTransForDocument(document);
         }
     }
+
+    private void clearStatsCacheForUpdatedDocument(HDocument document) {
+        versionStateCacheImpl.clearVersionStatsCache(document.getProjectIteration()
+                .getId());
+    }
 }

zanata-war/src/main/java/org/zanata/service/impl/VersionStateCacheImpl.java

@@ -28,14 +28,19 @@
 import org.jboss.seam.ScopeType;
 import org.jboss.seam.annotations.Create;
 import org.jboss.seam.annotations.Destroy;
+import org.jboss.seam.annotations.In;
 import org.jboss.seam.annotations.Name;
 import org.jboss.seam.annotations.Observer;
 import org.jboss.seam.annotations.Scope;
 import org.zanata.cache.CacheWrapper;
 import org.zanata.cache.EhcacheWrapper;
 import org.zanata.common.LocaleId;
+import org.zanata.dao.LocaleDAO;
 import org.zanata.dao.ProjectIterationDAO;
+import org.zanata.dao.TextFlowDAO;
 import org.zanata.events.TextFlowTargetStateEvent;
+import org.zanata.model.HLocale;
+import org.zanata.model.HTextFlow;
 import org.zanata.service.VersionLocaleKey;
 import org.zanata.service.VersionStateCache;
 import org.zanata.ui.model.statistic.WordStatistic;
@@ -53,6 +58,12 @@ public class VersionStateCacheImpl implements VersionStateCache {
     private static final String VERSION_STATISTIC_CACHE_NAME = BASE
             + ".versionStatisticCache";
 
+    @In
+    private LocaleDAO localeDAO;
+
+    @In
+    private TextFlowDAO textFlowDAO;
+
     private CacheManager cacheManager;
 
     private CacheWrapper<VersionLocaleKey, WordStatistic> versionStatisticCache;
@@ -89,10 +100,13 @@ public void textFlowStateUpdated(TextFlowTargetStateEvent event) {
                 new VersionLocaleKey(event.getProjectIterationId(),
                         event.getLocaleId());
         WordStatistic stats = versionStatisticCache.get(key);
+        HTextFlow textFlow = textFlowDAO.findById(event.getTextFlowId());
 
         if (stats != null) {
-            stats.decrement(event.getPreviousState(), 1);
-            stats.increment(event.getNewState(), 1);
+            stats.decrement(event.getPreviousState(),
+                    textFlow.getWordCount().intValue());
+            stats.increment(event.getNewState(),
+                    textFlow.getWordCount().intValue());
             versionStatisticCache.put(key, stats);
         }
     }
@@ -104,6 +118,15 @@ public WordStatistic getVersionStatistics(Long projectIterationId,
                 projectIterationId, localeId));
     }
 
+    @Override
+    public void clearVersionStatsCache(Long versionId) {
+        for (HLocale locale : localeDAO.findAll()) {
+            VersionLocaleKey key =
+                    new VersionLocaleKey(versionId, locale.getLocaleId());
+            versionStatisticCache.remove(key);
+        }
+    }
+
     private static class VersionStatisticLoader extends
             CacheLoader<VersionLocaleKey, WordStatistic> {
 

zanata-war/src/test/java/org/zanata/rest/service/ResourceServiceRestTest.java

@@ -23,6 +23,7 @@
 import org.zanata.service.impl.CopyTransServiceImpl;
 import org.zanata.service.impl.DocumentServiceImpl;
 import org.zanata.service.impl.LocaleServiceImpl;
+import org.zanata.service.impl.VersionStateCacheImpl;
 
 public class ResourceServiceRestTest extends ResourceTranslationServiceRestTest {
     private final Logger log = LoggerFactory
@@ -40,7 +41,8 @@ protected void prepareResources() {
         seamAutowire.use("session", getSession()).use("entityManager", getEm())
                 .use("identity", mockIdentity).useImpl(LocaleServiceImpl.class)
                 .useImpl(CopyTransServiceImpl.class)
-                .useImpl(DocumentServiceImpl.class);
+                .useImpl(DocumentServiceImpl.class)
+                .useImpl(VersionStateCacheImpl.class);
 
         SourceDocResourceService sourceDocResourceService =
                 seamAutowire.autowire(SourceDocResourceService.class);

zanata-war/src/test/java/org/zanata/rest/service/TranslationResourceRestTest.java

@@ -148,7 +148,8 @@ protected void prepareResources() {
                 .useImpl(DocumentServiceImpl.class)
                 .useImpl(ResourceUtils.class)
                 .useImpl(SecurityServiceImpl.class)
-                .useImpl(ValidationServiceImpl.class);
+                .useImpl(ValidationServiceImpl.class)
+                .useImpl(VersionStateCacheImpl.class);
 
         TranslatedDocResourceService translatedDocResourceService =
                 seamAutowire.autowire(TranslatedDocResourceService.class);

zanata-war/src/test/java/org/zanata/rest/service/TranslationServiceRestTest.java

@@ -56,7 +56,8 @@ protected void prepareResources() {
                 .useImpl(CopyTransServiceImpl.class)
                 .useImpl(DocumentServiceImpl.class)
                 .useImpl(TranslationServiceImpl.class)
-                .useImpl(ValidationServiceImpl.class);
+                .useImpl(ValidationServiceImpl.class)
+                .useImpl(VersionStateCacheImpl.class);
 
         SourceDocResourceService sourceDocResourceService =
                 seamAutowire.autowire(SourceDocResourceService.class);