Merge branch 'master' into fbn

common/memcache_decorator.py

@@ -34,7 +34,8 @@ def DefaultCreateKey(func, prefix, args, kwargs=None):
   Returns:
     str - the string to use as key in memcache.
   """
-  key = prefix or func.__name__
+  module = func.__module__ or 'None'
+  key = prefix or module + '.' + func.__name__
   if args:
     for arg in args:
       try:

docs/setup.md

@@ -59,7 +59,7 @@ then applies `UsernameToEmail`.
 Next, you need to give yourself admin rights to the application. To do so, you
 must modify the user grouping interface Upvote uses to assign elevated-privilege
 roles. Upvote provides a simple default implementation of user grouping (found
-in `GroupManager` in [groups.py](upvote/gae/shared/common/groups.py)) so all you
+in `GroupManager` in [groups.py](upvote/gae/utils/group_utils.py)) so all you
 need to do is add your email (and those of any other admins) to the
 '`admin-users`' group. This static solution should suffice for small
 organizations and for individual users.

upvote/gae/cron/BUILD

@@ -37,8 +37,8 @@ py_appengine_library(
         "//upvote/gae/datastore:utils",
         "//upvote/gae/datastore/models:santa",
         "//upvote/gae/datastore/models:user",
-        "//upvote/gae/shared/common:groups",
         "//upvote/gae/shared/common:user_map",
+        "//upvote/gae/utils:group_utils",
         "//upvote/gae/utils:iter_utils",
     ],
 )
@@ -80,8 +80,8 @@ upvote_appengine_test(
         "//upvote/gae/datastore:test_utils",
         "//upvote/gae/datastore/models:base",
         "//upvote/gae/lib/testing:basetest",
-        "//upvote/gae/shared/common:groups",
         "//upvote/gae/shared/common:settings",
+        "//upvote/gae/utils:group_utils",
         "//upvote/shared:constants",
     ],
 )

upvote/gae/cron/main.py

@@ -22,10 +22,12 @@
 from upvote.gae.cron import role_syncing
 
 _ALL_ROUTES = [
-    routes.PathPrefixRoute('/cron', [
-        datastore_backup.ROUTES,
-        role_syncing.ROUTES
-    ]),
+    routes.PathPrefixRoute(
+        '/cron',
+        [
+            datastore_backup.ROUTES,
+            role_syncing.ROUTES
+        ]),
 ]
 
 app = webapp2.WSGIApplication(routes=_ALL_ROUTES)

upvote/gae/cron/role_syncing.py

@@ -26,9 +26,9 @@
 from upvote.gae.datastore import utils as datastore_utils
 from upvote.gae.datastore.models import santa
 from upvote.gae.datastore.models import user as user_models
-from upvote.gae.shared.common import groups
 from upvote.gae.shared.common import settings
 from upvote.gae.shared.common import user_map
+from upvote.gae.utils import group_utils
 from upvote.gae.utils import iter_utils
 from upvote.shared import constants
 
@@ -43,7 +43,7 @@ def get(self):  # pylint: disable=g-bad-name
 
     logging.info('Starting role sync...')
     group_role_assignments = settings.GROUP_ROLE_ASSIGNMENTS
-    group_client = groups.GroupManager()
+    group_client = group_utils.GroupManager()
 
     # Iterate over the syncing dict, where each entry consists of a role key
     # which maps to a list of groups that should have that role.
@@ -123,7 +123,7 @@ def _ChangeModeForGroup(self, mode, group, honor_lock=True):
     """
     logging.info('Changing mode to %s for %s', mode, group)
 
-    group_client = groups.GroupManager()
+    group_client = group_utils.GroupManager()
     roster = group_client.AllMembers(group)
     logging.debug('Fetched %d user(s) from group %s', len(roster), group)
 

upvote/gae/cron/role_syncing_test.py

@@ -57,7 +57,7 @@ def VerifyUser(self, email_addr, expected_roles):
     expected_vote_weight = max(voting_weights[r] for r in expected_roles)
     self.assertEqual(expected_vote_weight, user.vote_weight)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_GroupDoesNotExist(self, mock_ctor):
     """Tests a sync with a nonexistent group."""
 
@@ -79,7 +79,7 @@ def testGet_GroupDoesNotExist(self, mock_ctor):
     self.VerifyUser(user1.email, [USER])
     self.VerifyUser(user2.email, [USER])
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_AddRole(self, mock_ctor):
     """Tests a new role being added to the syncing dict."""
 
@@ -103,7 +103,7 @@ def testGet_AddRole(self, mock_ctor):
 
     self.assertBigQueryInsertions([constants.BIGQUERY_TABLE.USER] * 2)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_ExistingRole_AddRole(self, mock_ctor):
     """Tests a new role being added alongside an existing role."""
 
@@ -128,7 +128,7 @@ def testGet_ExistingRole_AddRole(self, mock_ctor):
 
     self.assertBigQueryInsertions([constants.BIGQUERY_TABLE.USER] * 2)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_ExistingRole_AddGroup(self, mock_ctor):
     """Tests a new group being added to an existing role."""
 
@@ -156,7 +156,7 @@ def testGet_ExistingRole_AddGroup(self, mock_ctor):
 
     self.assertBigQueryInsertion(constants.BIGQUERY_TABLE.USER)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_RemoveRole(self, mock_ctor):
     """Tests a role being removed entirely."""
 
@@ -180,7 +180,7 @@ def testGet_RemoveRole(self, mock_ctor):
 
     self.assertBigQueryInsertions([constants.BIGQUERY_TABLE.USER] * 2)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_ExistingRole_RemoveGroup(self, mock_ctor):
     """Tests a single group being removed from a role which has multiple."""
 
@@ -207,7 +207,7 @@ def testGet_ExistingRole_RemoveGroup(self, mock_ctor):
 
     self.assertBigQueryInsertion(constants.BIGQUERY_TABLE.USER)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_MemberAdded(self, mock_ctor):
     """Tests a member being added to an existing group."""
 
@@ -234,7 +234,7 @@ def testGet_MemberAdded(self, mock_ctor):
 
     self.assertBigQueryInsertion(constants.BIGQUERY_TABLE.USER)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_MemberRemoved(self, mock_ctor):
     """Tests a member being removed from an existing group."""
 
@@ -262,7 +262,7 @@ def testGet_MemberRemoved(self, mock_ctor):
 
     self.assertBigQueryInsertion(constants.BIGQUERY_TABLE.USER)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testGet_EnsureFailsafeAdmins(self, mock_ctor):
 
     self.PatchSetting('GROUP_ROLE_ASSIGNMENTS', {ADMINISTRATOR: ['group1']})
@@ -304,7 +304,7 @@ def setUp(self):
         [webapp2.Route(r'', handler=FakeClientModeChangeHandler)])
     super(ClientModeChangeHandlerTest, self).setUp(wsgi_app=app)
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   @mock.patch.dict(role_syncing.__dict__, values={'BATCH_SIZE': 2})
   def testChangeModeForGroup_SingleBatch(self, mock_ctor):
 
@@ -328,7 +328,7 @@ def testChangeModeForGroup_SingleBatch(self, mock_ctor):
     new_hosts = ndb.get_multi(host.key for host in hosts)
     self.assertTrue(all(host.client_mode == LOCKDOWN for host in new_hosts))
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   @mock.patch.dict(role_syncing.__dict__, values={'BATCH_SIZE': 2})
   def testChangeModeForGroup_MultiBatch(self, mock_ctor):
 
@@ -352,7 +352,7 @@ def testChangeModeForGroup_MultiBatch(self, mock_ctor):
     new_hosts = ndb.get_multi(host.key for host in hosts)
     self.assertTrue(all(host.client_mode == LOCKDOWN for host in new_hosts))
 
-  @mock.patch.object(role_syncing.groups, 'GroupManager')
+  @mock.patch.object(role_syncing.group_utils, 'GroupManager')
   def testChangeModeForGroup_NoUsers(self, mock_ctor):
 
     mock_ctor.return_value.AllMembers.return_value = []

upvote/gae/datastore/models/base.py

@@ -16,9 +16,7 @@
 import datetime
 import hashlib
 import logging
-import re
 
-from google.appengine.api import memcache
 from google.appengine.ext import ndb
 from google.appengine.ext.ndb import polymodel
 
@@ -31,10 +29,6 @@
 from upvote.shared import constants
 
 
-_BLACKLIST_MEMCACHE_KEY = '_blacklist'
-_BLACKLIST_MEMCACHE_EXPIRATION = 3600  # in seconds
-
-
 # Done for the sake of brevity.
 LOCAL = constants.RULE_SCOPE.LOCAL
 GLOBAL = constants.RULE_SCOPE.GLOBAL
@@ -514,55 +508,6 @@ def IsAssociatedWithUser(self, user):
     """
     raise NotImplementedError
 
-  def GetUserBlockRate(
-      self, user, duration_to_fetch=datetime.timedelta(days=60),
-      max_events_to_fetch=1000):
-    """Calculates the block rate for a given user on this host.
-
-    "Block rate" is defined as the number of _unique_ blockables a user runs on
-    the host every _workday_ (i.e. 5 out of 7 days per week).
-
-    Args:
-      user: User, The user for whom to calculate the block rate on this
-          host.
-      duration_to_fetch: datetime.timedelta, The span of time over which the
-          block rate should be calculated.
-      max_events_to_fetch: int, The maximum number of events to be counted. The
-          mitigates the risk that a host with thousands of events results in the
-          datastore query timing out.
-
-    Returns:
-      (bool, float), A 2-tuple of the form (was_max, block_rate). was_max is
-      True when max_events_to_fetch events were found in the provided time
-      frame. block_rate is the block rate for the given user on this host.
-
-    Raises:
-      InvalidArgumentError: duration_to_fetch is less than 1 day or
-          max_events_to_fetch is less than 1.
-    """
-    # Duration must be at least 1 day.
-    if duration_to_fetch.days == 0:
-      raise InvalidArgumentError('Duration must be at least 1 day')
-    elif max_events_to_fetch <= 0:
-      raise InvalidArgumentError('Max Events must be at least 1')
-
-    threshold_dt = datetime.datetime.utcnow() - duration_to_fetch
-    parent_key = model_utils.ConcatenateKeys(user.key, self.key)
-    query = Event.query(
-        Event.last_blocked_dt >= threshold_dt,
-        ancestor=parent_key
-    ).order(-Event.last_blocked_dt)
-
-    num_events = query.count(limit=max_events_to_fetch)
-
-    was_max = num_events == max_events_to_fetch
-
-    # 5 workdays out of 7 days of the week.
-    ratio_of_workdays = 5. / 7
-    workdays_to_fetch = ratio_of_workdays * duration_to_fetch.days
-    block_rate = float(num_events) / workdays_to_fetch
-    return (was_max, block_rate)
-
   @staticmethod
   def NormalizeId(host_id):
     return host_id.upper()
@@ -662,32 +607,3 @@ def InsertBigQueryRow(self, **kwargs):
     defaults.update(kwargs.copy())
 
     tables.RULE.InsertRow(**defaults)
-
-
-class Blacklist(ndb.Model):
-  """Model for storing regular expressions for items that should be blacklisted.
-
-  Attributes:
-    regex: str, regular expression whose matches should be blacklisted.
-    updated_dt: datetime, when the state was last changed.
-  """
-  regex = ndb.StringProperty()
-  updated_dt = ndb.DateTimeProperty(auto_now=True)
-
-  @classmethod
-  def GetBlacklist(cls):
-    """Returns all Blacklist entries."""
-    entries = memcache.get(_BLACKLIST_MEMCACHE_KEY) or []
-    if not entries:
-      entries = Blacklist.query().fetch()
-      memcache.set(
-          _BLACKLIST_MEMCACHE_KEY, entries, time=_BLACKLIST_MEMCACHE_EXPIRATION)
-    return entries
-
-  @classmethod
-  def IsBlacklisted(cls, text):
-    """Returns True if the text matches a blacklist entry, False otherwise."""
-    for entry in Blacklist.GetBlacklist():
-      if re.match(entry.regex, text):
-        return True
-    return False

upvote/gae/datastore/models/base_test.py

@@ -19,7 +19,6 @@
 import mock
 
 from google.appengine.ext import ndb
-from google.appengine.ext import testbed
 
 from upvote.gae.datastore import test_utils
 from upvote.gae.datastore import utils
@@ -426,42 +425,6 @@ def testToDict(self):
     self.assertIn('cert_id', self.blockable.to_dict())
 
 
-class BlacklistTest(basetest.UpvoteTestCase):
-
-  def setUp(self):
-
-    self.testbed = testbed.Testbed()
-
-    self.testbed.activate()
-    self.testbed.init_datastore_v3_stub()
-    self.testbed.init_memcache_stub()
-
-    regexes = [
-        r'.*7-Zip GUI.*',
-        r'.*Firefox.*',
-        r'.*Windows PowerShell.*',
-        r'.*WinZip 17\.5 Setup.*',
-        r'.*\.dll.*']
-
-    for regex in regexes:
-      base.Blacklist(regex=regex).put()
-
-  def tearDown(self):
-    self.testbed.deactivate()
-
-  def testGetBlacklist(self):
-    blacklist = base.Blacklist.GetBlacklist()
-    self.assertEqual(5, len(blacklist))
-
-  def testIsBlacklisted(self):
-    self.assertTrue(base.Blacklist.IsBlacklisted('7-Zip GUI 2.0'))
-    self.assertTrue(base.Blacklist.IsBlacklisted('Something Firefox Something'))
-    self.assertTrue(base.Blacklist.IsBlacklisted('Fancy Windows PowerShell'))
-    self.assertTrue(base.Blacklist.IsBlacklisted('WinZip 17.5 Setup Thing'))
-    self.assertTrue(base.Blacklist.IsBlacklisted('not_malware.dll'))
-    self.assertFalse(base.Blacklist.IsBlacklisted('Not Malware For Real'))
-
-
 class HostTest(basetest.UpvoteTestCase):
 
   def setUp(self):
@@ -479,34 +442,6 @@ def testIsAssociatedWithUser(self):
     with self.assertRaises(NotImplementedError):
       self.host.IsAssociatedWithUser(self.user1)
 
-  def testGetUserBlockRate(self):
-    test_utils.CreateEvent(
-        self.blockable, last_blocked_dt=datetime.datetime.utcnow(),
-        parent=utils.ConcatenateKeys(self.user1.key, self.host.key))
-
-    was_max, block_rate = self.host.GetUserBlockRate(
-        self.user1, duration_to_fetch=datetime.timedelta(days=7))
-
-    self.assertFalse(was_max)
-    self.assertEqual(1. / 5, block_rate)
-
-  def testGetUserBlockRate_WasMax(self):
-    test_utils.CreateEvent(
-        self.blockable, last_blocked_dt=datetime.datetime.utcnow(),
-        parent=utils.ConcatenateKeys(self.user1.key, self.host.key))
-
-    was_max, _ = self.host.GetUserBlockRate(
-        self.user1, max_events_to_fetch=1)
-
-    self.assertTrue(was_max)
-
-  def testGetUserBlockRate_InvalidArgument(self):
-    with self.assertRaises(base.InvalidArgumentError):
-      self.host.GetUserBlockRate(
-          self.user1, duration_to_fetch=datetime.timedelta(seconds=1))
-    with self.assertRaises(base.InvalidArgumentError):
-      self.host.GetUserBlockRate(self.user1, max_events_to_fetch=0)
-
 
 class VoteTest(basetest.UpvoteTestCase):