Experimental commit to debug test.

lunrong · Apr 15, 2015 · 9e36f62 · 9e36f62
1 parent 25c6b25
commit 9e36f62
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 44 deletions.
diff --git a/core/domain/feedback_jobs.py b/core/domain/feedback_jobs.py
@@ -1,4 +1,20 @@
-"""Jobs for open feedbacks."""
+# coding: utf-8
+#
+# Copyright 2015 The Oppia Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS-IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Jobs for open feedback threads."""
 
 from core import jobs
 from core.platform import models
@@ -47,26 +63,23 @@ def get_num_of_open_feedbacks(cls, exploration_id):
         """
         Args:
           - exploration_id: id of the exploration to get statistics for
-          - exploration_version: str. Which version of the exploration to get
-              statistics for; this can be a version number, the string 'all',
-              or the string 'none'.
 
         Returns the number of open feedbacks.
         """
 
-        exp_model = user_models.OpenFeedbacksModel.get(
+        feedback_thread_analytics_model = feedback_models.OpenFeedbacksModel.get(
             exploration_id, strict=False)
-        return exp_model.num_of_open_feedbacks if exp_model else None
+        return feedback_thread_analytics_model.num_of_open_feedbacks \
+            if exp_model else None
 
 
 class OpenFeedbacksMRJobManager(
         jobs.BaseMapReduceJobManagerForContinuousComputations):
+    """Job that creates OpenFeedbackModels for explorations by calculating the
+       number of open feedback threads per exploration.
+       Includes:
+       * number of open feedbacks for an exploration.
     """
-    Job that calculates and creates summary models for exploration view.
-    Includes: * number of open feedbacks for an exploration.
-    """
-
-    STATUS_CHOICES_OPEN = 'open'
 
     @classmethod
     def _get_continuous_computation_class(cls):
@@ -78,11 +91,10 @@ def entity_classes_to_map_over(cls):
 
     @staticmethod
     def map(item):
-        if (item.status == STATUS_CHOICES_OPEN):
-            feedback_models.OpenFeedbacksModel.create_or_update(
-                item.id.split('.')[0])
-
+        if (item.status == feedback_models.STATUS_CHOICES_OPEN):
+            yield ('%s:%s' % (item.exploration_id, 1))
 
     @staticmethod
     def reduce(key, stringified_values):
-        pass
+        feedback_models.OpenFeedbacksModel.create(
+                key, len(stringified_values))
diff --git a/core/domain/feedback_jobs_test.py b/core/domain/feedback_jobs_test.py
@@ -0,0 +1,78 @@
+# coding: utf-8
+#
+# Copyright 2014 The Oppia Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS-IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Tests for open feedback threads continuous computations."""
+
+from core import jobs_registry
+from core.domain import event_services
+from core.domain import feedback_jobs
+from core.platform import models
+(feedback_models,) = models.Registry.import_models([models.NAMES.feedback])
+from core.tests import test_utils
+import feconf
+
+
+class ModifiedOpenFeedbacksAggregator(feedback_jobs.OpenFeedbacksStatisticsAggregator):
+    """A modified OpenFeedbacksStatisticsAggregator that does not start a new batch
+    job when the previous one has finished.
+    """
+    @classmethod
+    def _get_batch_job_manager_class(cls):
+        return ModifiedOpenFeedbacksMRJobManager
+
+    @classmethod
+    def _kickoff_batch_job_after_previous_one_ends(cls):
+        pass
+
+
+class ModifiedOpenFeedbacksMRJobManager(feedback_jobs.OpenFeedbacksMRJobManager):
+
+    @classmethod
+    def _get_continuous_computation_class(cls):
+        return ModifiedOpenFeedbacksStatisticsAggregator
+
+
+class OpenFeedbaksAggregatorUnitTests(test_utils.GenericTestBase):
+    """Tests for statistics aggregations."""
+
+    ALL_CONTINUOUS_COMPUTATION_MANAGERS_FOR_TESTS = [
+        ModifiedOpenFeedbacksAggregator]
+
+    def test_multiple_threads_open(self):
+        with self.swap(
+                jobs_registry, 'ALL_CONTINUOUS_COMPUTATION_MANAGERS',
+                self.ALL_CONTINUOUS_COMPUTATION_MANAGERS_FOR_TESTS):
+            # Create test objects.
+            exp_id = 'eid'
+            thread_id_1 = 'tid1'
+            thread_id_2 = 'tid2'
+            self.save_new_valid_exploration(exp_id, 'owner')
+            thread_1= feedback_models.FeedbackThreadModel.create(
+                exp_id, thread_id_1)
+            thread_1.exploration_id = exp_id
+            thread_1.put()
+            thread_2 = feedback_models.FeedbackThreadModel.create(
+                exp_id, thread_id_2)
+            thread_2.exploration_id = exp_id          
+            thread_2.put()
+
+            self.process_and_flush_pending_tasks()
+            ModifiedOpenFeedbacksAggregator.start_computation()
+            self.assertEqual(self.count_jobs_in_taskqueue(), 1)
+            self.process_and_flush_pending_tasks()
+
+            output_model = feedback_models.OpenFeedbacksModel.get(exp_id)
+            self.assertEqual(output_model.num_of_open_feedbacks, 2)
diff --git a/core/domain/feedback_services.py b/core/domain/feedback_services.py
@@ -99,10 +99,6 @@ def create_message(
     msg.author_id = author_id
     if updated_status:
         msg.updated_status = updated_status
-    if (thread.status = feedback_models.STATUS_CHOICES_OPEN 
-        and updated_status in feedback_models.STATUS_CHOICES
-        and updated_status != feedback_models.STATUS_CHOICES_OPEN)
-        feedback_closure_update(thread.exploration_id)
     if updated_subject:
         msg.updated_subject = updated_subject
     msg.text = text
@@ -156,11 +152,5 @@ def get_open_feedbacks(exploration_id):
 
     If this exploration has no open feedbacks, returns None.
     """
-    return feedback_jobs.OpenFeedbacksStatisticsAggregator.
+    return feedback_jobs.OpenFeedbacksStatisticsAggregator. \
         get_num_of_open_feedbacks(exploration_id)
-
-
-def feedback_closure_update(exploration_id):
-    """Decrements the number of open feedbacks for this exploration when a
-    feedback is closed."""
-    feedback_models.OpenFeedbacksModel.update(exploration_id)
diff --git a/core/jobs_registry.py b/core/jobs_registry.py
@@ -21,6 +21,7 @@
 from core.domain import exp_jobs
 from core.domain import stats_jobs
 from core.domain import user_jobs
+from core.domain import feedback_jobs
 
 # List of all manager classes for one-off batch jobs for which to show controls
 # on the admin dashboard.
@@ -37,7 +38,8 @@
 ALL_CONTINUOUS_COMPUTATION_MANAGERS = [
     exp_jobs.SearchRanker,
     stats_jobs.StatisticsAggregator,
-    user_jobs.DashboardRecentUpdatesAggregator]
+    user_jobs.DashboardRecentUpdatesAggregator,
+    feedback_jobs.OpenFeedbacksStatisticsAggregator]
 
 
 class ContinuousComputationEventDispatcher(object):

diff --git a/core/storage/feedback/gae_models.py b/core/storage/feedback/gae_models.py
@@ -206,7 +206,7 @@ def get_all_messages(cls, page_size, urlsafe_start_cursor):
 
 
 class OpenFeedbacksModel(base_models.BaseModel):
-    """Model for storing open feedback counts for explorations  .
+    """Model for storing open feedback counts for explorations.
 
     A OpenFeedbacksModel instance stores the following information:
 
@@ -216,20 +216,9 @@ class OpenFeedbacksModel(base_models.BaseModel):
     """
 
     # The number of open feedbacks for an exploration.
-    num_of_open_feedbacks = ndb.IntegerProperty(default=None, indexed=True)    
+    num_of_open_feedbacks = ndb.IntegerProperty(default=None, indexed=False)    
 
     @classmethod
-    def create_or_update(cls, id):
-      entry = cls.get_by_id(id, strict=false)
-      if entry is None:
-          cls(id=id, num_of_open_feedbacks=1).put()
-      else:
-        entry.num_of_open_feedbacks += 1
-        entry.put()
-
-    @classmethod
-    def update(cls, id):
-      entry = cls.get_by_id(id, strict=false)
-      if entry is not NONE
-          entry.num_of_open_feedbacks -= 1
-          entry.put()
+    def create(cls, id, open_feedbacks):
+        """Creates a new OpenFeedbacksModel entry."""
+        cls(id=id, num_of_open_feedbacks=open_feedbacks).put()
diff --git a/index.yaml b/index.yaml
@@ -71,6 +71,11 @@ indexes:
   - name: time_queued_msec
     direction: desc
 
+- kind: OpenFeedbacksRealtimeModel
+  properties:
+  - name: realtime_layer
+  - name: created_on
+
 - kind: RecentUpdatesRealtimeModel
   properties:
   - name: realtime_layer