refactor(ZNTA-1017): translation updates event to handle batch (#1138)

refactor(ZNTA-1017): translation updates event to handle batch

https://zanata.atlassian.net/browse/ZNTA-1017

* Use ImmutableList for event

* Refactor on code review comments

* Refactor activity and translation cache handling

* Remove Hashmap in TranslationServiceImpl saveBatch method, rename variables

* Remove entry from release notes

* Move projectIterationId to event class from DocumentLocaleKey

docs/release-notes.md

@@ -31,7 +31,6 @@
 * [ZNTA-412](https://zanata.atlassian.net/browse/ZNTA-412) - Profile link to project maintainers, language members, and version group maintainers
 * [ZNTA-905](https://zanata.atlassian.net/browse/ZNTA-905) - Remove 0% matching translation memory entry
 * [ZNTA-928](https://zanata.atlassian.net/browse/ZNTA-928) - Readonly project doesn't have "lock" icon in UI
-* [ZNTA-793](https://zanata.atlassian.net/browse/ZNTA-793) - Remove unused method in TranslationMemoryAction
 * [ZNTA-54](https://zanata.atlassian.net/browse/ZNTA-54) - Allow delete language
 
 <h5>Infrastructure Changes</h5>

zanata-war/src/main/java/org/zanata/action/VersionHomeAction.java

@@ -44,6 +44,7 @@
 import javax.inject.Named;
 import org.richfaces.event.FileUploadEvent;
 import org.richfaces.model.UploadedFile;
+import org.zanata.events.DocumentLocaleKey;
 import org.zanata.exception.AuthorizationException;
 import org.zanata.async.handle.CopyVersionTaskHandle;
 import org.zanata.common.DocumentType;
@@ -543,14 +544,6 @@ public void setPageRendered(boolean pageRendered) {
         this.pageRendered = pageRendered;
     }
 
-    @Getter
-    @AllArgsConstructor
-    @EqualsAndHashCode
-    public class DocumentLocaleKey {
-        private Long documentId;
-        private LocaleId localeId;
-    }
-
     public WordStatistic getStatisticsForLocale(LocaleId localeId) {
         return localeStatisticMap.get(localeId);
     }

zanata-war/src/main/java/org/zanata/events/DocumentLocaleKey.java

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2016, Red Hat, Inc. and individual contributors as indicated by the
+ * @author tags. See the copyright.txt file in the distribution for a full
+ * listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it under the
+ * terms of the GNU Lesser General Public License as published by the Free
+ * Software Foundation; either version 2.1 of the License, or (at your option)
+ * any later version.
+ *
+ * This software is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this software; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF
+ * site: http://www.fsf.org.
+ */
+
+package org.zanata.events;
+
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.zanata.common.LocaleId;
+
+import java.io.Serializable;
+
+/**
+ * @author Alex Eng<a href="mailto:aeng@redhat.com">aeng@redhat.com</a>
+ */
+@AllArgsConstructor
+@EqualsAndHashCode
+@Getter
+public class DocumentLocaleKey implements Serializable {
+
+    private final Long documentId;
+    private final LocaleId localeId;
+}

zanata-war/src/main/java/org/zanata/events/TextFlowTargetStateEvent.java

@@ -21,27 +21,48 @@
 
 package org.zanata.events;
 
-import javax.annotation.Nullable;
+import java.io.Serializable;
 
-import lombok.Value;
+import com.google.common.collect.ImmutableList;
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
 import org.zanata.common.ContentState;
-import org.zanata.common.LocaleId;
+
+import javax.annotation.Nullable;
 
 /**
  * @author Sean Flanigan <a
  *         href="mailto:sflaniga@redhat.com">sflaniga@redhat.com</a>
  *
  */
-@Value
+@AllArgsConstructor
 public final class TextFlowTargetStateEvent {
-    // this may be null in the case of document uploads
-    private final @Nullable
-    Long actorId;
+    @Getter
+    private final DocumentLocaleKey key;
+
+    @Getter
     private final Long projectIterationId;
-    private final Long documentId;
-    private final Long textFlowId;
-    private final LocaleId localeId;
-    private final Long textFlowTargetId;
-    private final ContentState newState;
-    private final ContentState previousState;
+
+    @Getter
+    // this may be null in the case of document uploads
+    private final @Nullable Long actorId;
+
+    @Getter
+    private final ImmutableList<TextFlowTargetState> states;
+
+    public TextFlowTargetStateEvent(DocumentLocaleKey key,
+        Long projectIterationId, Long actorId, TextFlowTargetState state) {
+        this(key, projectIterationId, actorId, ImmutableList.of(state));
+    }
+
+    @Getter
+    @EqualsAndHashCode
+    @AllArgsConstructor
+    public static final class TextFlowTargetState implements Serializable {
+        private final Long textFlowId;
+        private final Long textFlowTargetId;
+        private final ContentState newState;
+        private final ContentState previousState;
+    }
 }

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

@@ -24,6 +24,7 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.locks.Lock;
 
 import javax.enterprise.context.RequestScoped;
@@ -32,6 +33,8 @@
 import javax.persistence.EntityManager;
 
 import com.google.common.base.Throwables;
+import com.google.common.collect.Maps;
+import org.apache.commons.collections.map.HashedMap;
 import org.apache.commons.lang.time.DateUtils;
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -184,23 +187,54 @@ public Object getEntity(EntityType entityType, long entityId) {
     public void logTextFlowStateUpdate(@Observes(during = TransactionPhase.AFTER_SUCCESS) TextFlowTargetStateEvent event_) {
         // workaround for https://issues.jboss.org/browse/WELD-2019
         final TextFlowTargetStateEvent event = event_;
+
         Long actorId = event.getActorId();
+
         if (actorId != null) {
             Lock lock = activityLockManager.getLock(actorId);
             lock.lock();
             try {
                 transactionUtil.run(() -> {
-                    HTextFlowTarget target =
-                            textFlowTargetDAO.findById(event.getTextFlowTargetId(),
-                                    false);
-                    HDocument document = documentDAO.getById(event.getDocumentId());
-                    ActivityType activityType =
-                            event.getNewState().isReviewed() ? ActivityType.REVIEWED_TRANSLATION
-                                    : ActivityType.UPDATE_TRANSLATION;
-
-                    logActivityAlreadyLocked(actorId,
-                            document.getProjectIteration(), target, activityType,
-                            target.getTextFlow().getWordCount().intValue());
+                    HDocument document =
+                        documentDAO.getById(event.getKey().getDocumentId());
+
+                    HTextFlowTarget lastReviewedTarget = null;
+                    HTextFlowTarget lastTranslatedTarget = null;
+
+                    int totalReviewedWords = 0;
+                    int totalTranslatedWords = 0;
+
+                    for (TextFlowTargetStateEvent.TextFlowTargetState state : event
+                        .getStates()) {
+                        HTextFlowTarget target =
+                            textFlowTargetDAO.findById(
+                                state.getTextFlowTargetId(), false);
+                        if (state.getNewState().isReviewed()) {
+                            lastReviewedTarget = target;
+                            totalReviewedWords +=
+                                target.getTextFlow().getWordCount()
+                                    .intValue();
+                        } else {
+                            lastTranslatedTarget = target;
+                            totalTranslatedWords +=
+                                target.getTextFlow().getWordCount()
+                                    .intValue();
+                        }
+                    }
+                    if (lastReviewedTarget != null) {
+                        logActivityAlreadyLocked(actorId,
+                            document.getProjectIteration(), lastReviewedTarget,
+                            ActivityType.REVIEWED_TRANSLATION,
+                            totalReviewedWords);
+                    }
+
+                    if (lastTranslatedTarget != null) {
+                        logActivityAlreadyLocked(actorId,
+                            document.getProjectIteration(),
+                            lastTranslatedTarget,
+                            ActivityType.UPDATE_TRANSLATION,
+                            totalTranslatedWords);
+                    }
                 });
             } catch (Exception e) {
                 Throwables.propagate(e);

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

@@ -38,7 +38,9 @@
 import javax.inject.Named;
 import org.zanata.common.ContentState;
 import org.zanata.dao.TextFlowTargetDAO;
+import org.zanata.events.DocumentLocaleKey;
 import org.zanata.events.TextFlowTargetStateEvent;
+import org.zanata.model.HAccount;
 import org.zanata.model.HCopyTransOptions;
 import org.zanata.model.HDocument;
 import org.zanata.model.HLocale;
@@ -47,6 +49,7 @@
 import org.zanata.model.HTextFlow;
 import org.zanata.model.HTextFlowTarget;
 import org.zanata.model.type.TranslationSourceType;
+import org.zanata.security.annotations.Authenticated;
 import org.zanata.service.ValidationService;
 import org.zanata.service.VersionStateCache;
 import org.zanata.util.TranslationUtil;
@@ -55,6 +58,7 @@
 import com.google.common.base.Optional;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 
 /**
  * @author Sean Flanigan <a href="mailto:sflaniga@redhat.com">sflaniga@redhat.com</a>
@@ -78,6 +82,9 @@ public class CopyTransWorkFactory {
     @Inject
     private VersionStateCache versionStateCacheImpl;
 
+    @Inject @Authenticated
+    private HAccount authenticatedAccount;
+
     public Integer runCopyTransInNewTx(HLocale targetLocale,
             HCopyTransOptions options, HDocument document,
             boolean requireTranslationReview, List<HTextFlow> copyTargets)
@@ -104,6 +111,7 @@ public Integer runCopyTrans(HLocale targetLocale,
             checkContext = true;
         }
 
+        Long actorId = authenticatedAccount.getPerson().getId();
         for (HTextFlow textFlow : copyTargets) {
             if (shouldFindMatch(textFlow, targetLocale,
                     requireTranslationReview)) {
@@ -116,8 +124,8 @@ public Integer runCopyTrans(HLocale targetLocale,
                 if (bestMatch.isPresent()) {
                     numCopied++;
 
-                    saveCopyTransMatch(bestMatch.get(), textFlow, options,
-                            requireTranslationReview);
+                    saveCopyTransMatch(actorId, bestMatch.get(), textFlow,
+                        options, requireTranslationReview);
 
                 }
             }
@@ -220,9 +228,10 @@ static ContentState determineContentState(Supplier<Boolean> contextMatches,
                 requireTranslationReview, matchingTargetState);
     }
 
-    private void saveCopyTransMatch(final HTextFlowTarget matchingTarget,
-            final HTextFlow originalTf, final HCopyTransOptions options,
-            final boolean requireTranslationReview) {
+    private void saveCopyTransMatch(Long actorId,
+        final HTextFlowTarget matchingTarget,
+        final HTextFlow originalTf, final HCopyTransOptions options,
+        final boolean requireTranslationReview) {
         final HProjectIteration matchingTargetProjectIteration =
                 matchingTarget.getTextFlow().getDocument()
                         .getProjectIteration();
@@ -306,7 +315,7 @@ public Boolean get() {
 
             // TODO Maybe we should think about registering a Hibernate
             // integrator for these updates
-            signalCopiedTranslation(hTarget, prevState);
+            signalCopiedTranslation(actorId, hTarget, prevState);
         }
     }
 
@@ -392,7 +401,7 @@ private static boolean shouldOverwrite(HTextFlowTarget currentlyStored,
         return true;
     }
 
-    private void signalCopiedTranslation(HTextFlowTarget target,
+    private void signalCopiedTranslation(Long actorId, HTextFlowTarget target,
             ContentState previousState) {
         /*
          * Using a direct method call instead of an event because it's easier to
@@ -403,12 +412,21 @@ private void signalCopiedTranslation(HTextFlowTarget target,
         // TODO how was this not causing duplicate events?  Is this bypassing TranslationServiceImpl?
         // FIXME other observers may not be notified
         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);
+
+        DocumentLocaleKey key =
+            new DocumentLocaleKey(document.getId(), target.getLocaleId());
+
+        TextFlowTargetStateEvent.TextFlowTargetState state =
+            new TextFlowTargetStateEvent.TextFlowTargetState(
+                target.getTextFlow().getId(),
+                target.getId(), target.getState(),
+                previousState);
+
+        TextFlowTargetStateEvent event =
+            new TextFlowTargetStateEvent(key,
+                document.getProjectIteration().getId(), actorId, state);
+
+        versionStateCacheImpl.textFlowStateUpdated(event);
     }
 
     /**