Merge branch 'main' into avm99963-monorail
Merged commit 3779da353b36d43cf778e7d4f468097714dd4540
GitOrigin-RevId: 6451a5c6b75afb0fd1f37b3f14521148d0722ea8
diff --git a/framework/banned.py b/framework/banned.py
index 231f76f..209a715 100644
--- a/framework/banned.py
+++ b/framework/banned.py
@@ -22,7 +22,7 @@
from framework import servlet
-class Banned(servlet.Servlet):
+class Banned(flaskservlet.FlaskServlet):
"""The Banned page shows a message explaining that the user is banned."""
_PAGE_TEMPLATE = 'framework/banned-page.ezt'
@@ -53,5 +53,5 @@
'currentPageURLEncoded': None,
}
- # def GetNoAccessPage(self, **kwargs):
- # return self.handler(**kwargs)
+ def GetNoAccessPage(self, **kwargs):
+ return self.handler(**kwargs)
diff --git a/framework/clientmon.py b/framework/clientmon.py
index f512f4d..fd10684 100644
--- a/framework/clientmon.py
+++ b/framework/clientmon.py
@@ -19,8 +19,7 @@
from infra_libs import ts_mon
-# TODO: convert to FlaskJsonFeed while convert to Flask
-class ClientMonitor(jsonfeed.JsonFeed):
+class ClientMonitor(jsonfeed.FlaskJsonFeed):
"""JSON feed to track client side js errors in ts_mon."""
js_errors = ts_mon.CounterMetric('frontend/js_errors',
@@ -37,9 +36,7 @@
Dict of values used by EZT for rendering the page.
"""
- # TODO: uncomment while convert to flask
- # post_data = mr.request.values
- post_data = mr.request.POST
+ post_data = mr.request.values
errors = post_data.get('errors')
try:
errors = json.loads(errors)
@@ -55,8 +52,5 @@
return {}
- # def GetClientMonitor(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostClientMonitor(self, **kwargs):
- # return self.handler(**kwargs)
+ def PostClientMonitor(self, **kwargs):
+ return self.handler(**kwargs)
diff --git a/framework/csp_report.py b/framework/csp_report.py
index 83e3126..4b6f29e 100644
--- a/framework/csp_report.py
+++ b/framework/csp_report.py
@@ -11,12 +11,10 @@
from __future__ import division
from __future__ import absolute_import
-import webapp2
+import flask
import logging
-class CSPReportPage(webapp2.RequestHandler):
+def postCsp():
"""CSPReportPage serves CSP violation reports."""
-
- def post(self):
- logging.error('CSP Violation: %s' % self.request.body)
+ logging.error('CSP Violation: %s' % flask.request.get_data(as_text=True))
diff --git a/framework/deleteusers.py b/framework/deleteusers.py
index 739782e..015fad4 100644
--- a/framework/deleteusers.py
+++ b/framework/deleteusers.py
@@ -32,8 +32,7 @@
return credentials.authorize(httplib2.Http(timeout=60))
-# TODO: change to FlaskInternalTask when convert to Flask
-class WipeoutSyncCron(jsonfeed.InternalTask):
+class WipeoutSyncCron(jsonfeed.FlaskInternalTask):
"""Enqueue tasks for sending user lists to wipeout-lite and deleting deleted
users fetched from wipeout-lite."""
@@ -62,15 +61,11 @@
cloud_tasks_helpers.create_task(
task, queue=framework_constants.QUEUE_FETCH_WIPEOUT_DELETED_USERS)
- # def GetWipeoutSyncCron(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostWipeoutSyncCron(self, **kwargs):
- # return self.handler(**kwargs)
+ def GetWipeoutSyncCron(self, **kwargs):
+ return self.handler(**kwargs)
-# TODO: Set to FlaskInternalTask when convert
-class SendWipeoutUserListsTask(jsonfeed.InternalTask):
+class SendWipeoutUserListsTask(jsonfeed.FlaskInternalTask):
"""Sends a batch of monorail users to wipeout-lite."""
def HandleRequest(self, mr):
@@ -95,15 +90,11 @@
logging.info(
'Received response, %s with contents, %s', resp, data)
- # def GetSendWipeoutUserListsTask(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostSendWipeoutUserListsTask(self, **kwargs):
- # return self.handler(**kwargs)
+ def PostSendWipeoutUserListsTask(self, **kwargs):
+ return self.handler(**kwargs)
-# TODO: Set to FlaskInternalTask when convert
-class DeleteWipeoutUsersTask(jsonfeed.InternalTask):
+class DeleteWipeoutUsersTask(jsonfeed.FlaskInternalTask):
"""Fetches deleted users from wipeout-lite and enqueues tasks to delete
those users from Monorail's DB."""
@@ -137,15 +128,11 @@
'Received response, %s with contents, %s', resp, data)
return json.loads(data)
- # def GetDeleteWipeoutUsersTask(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostDeleteWipeoutUsersTask(self, **kwargs):
- # return self.handler(**kwargs)
+ def PostDeleteWipeoutUsersTask(self, **kwargs):
+ return self.handler(**kwargs)
-# TODO: Set to FlaskInternalTask when convert
-class DeleteUsersTask(jsonfeed.InternalTask):
+class DeleteUsersTask(jsonfeed.FlaskInternalTask):
"""Deletes users from Monorail's DB."""
def HandleRequest(self, mr):
@@ -160,8 +147,5 @@
with work_env.WorkEnv(mr, self.services) as we:
we.ExpungeUsers(emails, check_perms=False)
- # def GetDeleteUsersTask(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostDeleteUsersTask(self, **kwargs):
- # return self.handler(**kwargs)
+ def PostDeleteUsersTask(self, **kwargs):
+ return self.handler(**kwargs)
diff --git a/framework/excessiveactivity.py b/framework/excessiveactivity.py
index 0e54ebd..5506de3 100644
--- a/framework/excessiveactivity.py
+++ b/framework/excessiveactivity.py
@@ -12,18 +12,17 @@
from __future__ import division
from __future__ import absolute_import
-from framework import servlet
+from framework import flaskservlet
-class ExcessiveActivity(servlet.Servlet):
+class ExcessiveActivity(flaskservlet.FlaskServlet):
"""ExcessiveActivity page shows an error message."""
_PAGE_TEMPLATE = 'framework/excessive-activity-page.ezt'
# pylint: disable=unused-argument
def GetExcessiveActivity(self, **kwargs):
- return
- # return self.handler(**kwargs)
+ return self.handler(**kwargs)
def GatherPageData(self, _mr):
"""Build up a dictionary of data values to use when rendering the page."""
diff --git a/framework/flaskservlet.py b/framework/flaskservlet.py
index fce3eab..bc543d8 100644
--- a/framework/flaskservlet.py
+++ b/framework/flaskservlet.py
@@ -877,5 +877,5 @@
user_pb.last_visit_timestamp = now
self.services.user.UpdateUser(mr.cnxn, user_pb.user_id, user_pb)
- def abort(self, code, context):
+ def abort(self, code, context=""):
return flask.abort(code, context)
diff --git a/framework/jsonfeed.py b/framework/jsonfeed.py
index b7d85ac..1eff87d 100644
--- a/framework/jsonfeed.py
+++ b/framework/jsonfeed.py
@@ -171,7 +171,7 @@
if self.CHECK_SAME_APP and not settings.local_mode:
calling_app_id = request.headers.get('X-Appengine-Inbound-Appid')
if calling_app_id != app_identity.get_application_id():
- self.response.status = http_client.FORBIDDEN
+ self.response.status_code = http_client.FORBIDDEN
return
self._CheckForMovedProject(mr, request)
@@ -188,7 +188,7 @@
self.abort(400, msg)
except permissions.PermissionException as e:
logging.info('Trapped PermissionException %s', e)
- self.response.status = http_client.FORBIDDEN
+ self.response.status_code = http_client.FORBIDDEN
# pylint: disable=unused-argument
# pylint: disable=arguments-differ
diff --git a/framework/logger.py b/framework/logger.py
new file mode 100644
index 0000000..d2a8a0d
--- /dev/null
+++ b/framework/logger.py
@@ -0,0 +1,21 @@
+# Copyright 2022 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+""""Helper methods for structured logging."""
+
+from __future__ import print_function
+from __future__ import division
+from __future__ import absolute_import
+
+import google.cloud.logging
+
+import settings
+
+
+def log(struct):
+ if settings.local_mode or settings.unit_test_mode:
+ return
+
+ logging_client = google.cloud.logging.Client()
+ logger = logging_client.logger('python')
+ logger.log_struct(struct)
diff --git a/framework/monitoring.py b/framework/monitoring.py
index 6407e2d..08a1e23 100644
--- a/framework/monitoring.py
+++ b/framework/monitoring.py
@@ -4,13 +4,10 @@
"""Monitoring ts_mon custom to monorail."""
-import os
-import sys
-lib_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'lib')
-
-from google.cloud import logging
from infra_libs import ts_mon
+
from framework import framework_helpers
+from framework import logger
import settings
@@ -49,11 +46,9 @@
API_REQUESTS_COUNT.increment_by(1, fields)
if not settings.unit_test_mode:
- logging_client = logging.Client()
- logger = logging_client.logger("request_log")
- logger.log_struct(
+ logger.log(
{
- 'log_type': "IncrementAPIRequestsCount",
+ 'log_type': 'IncrementAPIRequestsCount',
'client_id': client_id,
'client_email': client_email,
'requests_count': str(API_REQUESTS_COUNT.get(fields)),
diff --git a/framework/reap.py b/framework/reap.py
index d0b721f..4654964 100644
--- a/framework/reap.py
+++ b/framework/reap.py
@@ -16,8 +16,7 @@
RUN_DURATION_LIMIT = 50 * 60 # 50 minutes
-# TODO: change to FlaskInternalTask when convert to Flask
-class Reap(jsonfeed.InternalTask):
+class Reap(jsonfeed.FlaskInternalTask):
"""Look for doomed and deletable projects and delete them."""
def HandleRequest(self, mr):
@@ -125,8 +124,5 @@
f(cnxn, project_id)
yield project_id
- # def GetReap(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostReap(self, **kwargs):
- # return self.handler(**kwargs)
+ def GetReap(self, **kwargs):
+ return self.handler(**kwargs)
diff --git a/framework/servlet.py b/framework/servlet.py
index 462939a..b363095 100644
--- a/framework/servlet.py
+++ b/framework/servlet.py
@@ -304,6 +304,8 @@
browser_major_version = int(ua['browser']['version'].split('.')[0])
except ValueError:
logging.warn('Could not parse version: %r', ua['browser']['version'])
+ except KeyError:
+ logging.warn('No browser version defined in user agent.')
csp_supports_report_sample = (
(browser == 'Chrome' and browser_major_version >= 59) or
(browser == 'Opera' and browser_major_version >= 46))
diff --git a/framework/sql.py b/framework/sql.py
index 1d7573d..0fb8043 100644
--- a/framework/sql.py
+++ b/framework/sql.py
@@ -23,6 +23,7 @@
from framework import exceptions
from framework import framework_helpers
+from framework import logger
from infra_libs import ts_mon
@@ -258,12 +259,20 @@
DB_RESULT_ROWS.add(cursor.rowcount)
if stmt_str.startswith('INSERT') or stmt_str.startswith('REPLACE'):
- formatted_statement = '%s %s' % (stmt_str, stmt_args)
+ formatted_statement = ('%s %s' % (stmt_str, stmt_args)).replace('\n', ' ')
else:
- formatted_statement = stmt_str % tuple(stmt_args)
+ formatted_statement = (stmt_str % tuple(stmt_args)).replace('\n', ' ')
logging.info(
'%d rows in %d ms: %s', cursor.rowcount, int(duration),
- formatted_statement.replace('\n', ' '))
+ formatted_statement)
+ if duration >= 2000:
+ logger.log({
+ 'log_type': 'database/query',
+ 'statement': formatted_statement,
+ 'type': formatted_statement.split(' ')[0],
+ 'duration': duration / 1000,
+ 'row_count': cursor.rowcount,
+ })
if commit and not stmt_str.startswith('SELECT'):
try:
diff --git a/framework/test/banned_test.py b/framework/test/banned_test.py
index 73b9f03..0331cdd 100644
--- a/framework/test/banned_test.py
+++ b/framework/test/banned_test.py
@@ -24,16 +24,16 @@
self.services = service_manager.Services()
def testAssertBasePermission(self):
- servlet = banned.Banned('request', 'response', services=self.services)
+ servlet = banned.Banned(services=self.services)
mr = monorailrequest.MonorailRequest(self.services)
mr.auth.user_id = 0 # Anon user cannot see banned page.
- with self.assertRaises(webapp2.HTTPException) as cm:
+ with self.assertRaises(Exception) as cm:
servlet.AssertBasePermission(mr)
self.assertEqual(404, cm.exception.code)
mr.auth.user_id = 111 # User who is not banned cannot view banned page.
- with self.assertRaises(webapp2.HTTPException) as cm:
+ with self.assertRaises(Exception) as cm:
servlet.AssertBasePermission(mr)
self.assertEqual(404, cm.exception.code)
@@ -42,7 +42,7 @@
servlet.AssertBasePermission(mr)
def testGatherPageData(self):
- servlet = banned.Banned('request', 'response', services=self.services)
+ servlet = banned.Banned(services=self.services)
self.assertNotEqual(servlet.template, None)
_request, mr = testing_helpers.GetRequestObjects()
diff --git a/framework/test/deleteusers_test.py b/framework/test/deleteusers_test.py
index 2609867..87ed5bc 100644
--- a/framework/test/deleteusers_test.py
+++ b/framework/test/deleteusers_test.py
@@ -25,8 +25,7 @@
def setUp(self):
self.services = service_manager.Services(user=fake.UserService())
- self.task = deleteusers.WipeoutSyncCron(
- request=None, response=None, services=self.services)
+ self.task = deleteusers.WipeoutSyncCron(services=self.services)
self.user_1 = self.services.user.TestAddUser('user1@example.com', 111)
self.user_2 = self.services.user.TestAddUser('user2@example.com', 222)
self.user_3 = self.services.user.TestAddUser('user3@example.com', 333)
@@ -100,8 +99,7 @@
def setUp(self):
self.services = service_manager.Services(user=fake.UserService())
- self.task = deleteusers.SendWipeoutUserListsTask(
- request=None, response=None, services=self.services)
+ self.task = deleteusers.SendWipeoutUserListsTask(services=self.services)
self.task.sendUserLists = mock.Mock()
deleteusers.authorize = mock.Mock(return_value='service')
self.user_1 = self.services.user.TestAddUser('user1@example.com', 111)
@@ -143,8 +141,7 @@
def setUp(self):
self.services = service_manager.Services()
deleteusers.authorize = mock.Mock(return_value='service')
- self.task = deleteusers.DeleteWipeoutUsersTask(
- request=None, response=None, services=self.services)
+ self.task = deleteusers.DeleteWipeoutUsersTask(services=self.services)
deleted_users = [
{'id': 'user1@gmail.com'}, {'id': 'user2@gmail.com'},
{'id': 'user3@gmail.com'}, {'id': 'user4@gmail.com'}]
diff --git a/framework/test/framework_helpers_test.py b/framework/test/framework_helpers_test.py
index 1d0146c..fb8810b 100644
--- a/framework/test/framework_helpers_test.py
+++ b/framework/test/framework_helpers_test.py
@@ -11,7 +11,10 @@
import mock
import unittest
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
import time
from businesslogic import work_env
diff --git a/framework/test/monorailcontext_test.py b/framework/test/monorailcontext_test.py
index ed93920..2071c9e 100644
--- a/framework/test/monorailcontext_test.py
+++ b/framework/test/monorailcontext_test.py
@@ -10,7 +10,10 @@
import unittest
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
from framework import authdata
from framework import monorailcontext
diff --git a/framework/test/monorailrequest_test.py b/framework/test/monorailrequest_test.py
index fcd30c3..ef52f1e 100644
--- a/framework/test/monorailrequest_test.py
+++ b/framework/test/monorailrequest_test.py
@@ -13,7 +13,10 @@
import re
import unittest
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
import six
from google.appengine.api import oauth
diff --git a/framework/test/permissions_test.py b/framework/test/permissions_test.py
index 0917b53..cd67c6c 100644
--- a/framework/test/permissions_test.py
+++ b/framework/test/permissions_test.py
@@ -11,7 +11,10 @@
import time
import unittest
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
import settings
from framework import authdata
diff --git a/framework/test/ratelimiter_test.py b/framework/test/ratelimiter_test.py
index b351f8c..84230e8 100644
--- a/framework/test/ratelimiter_test.py
+++ b/framework/test/ratelimiter_test.py
@@ -15,7 +15,10 @@
from google.appengine.api import memcache
from google.appengine.ext import testbed
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
import os
import settings
diff --git a/framework/test/reap_test.py b/framework/test/reap_test.py
index f1a907d..92d17fb 100644
--- a/framework/test/reap_test.py
+++ b/framework/test/reap_test.py
@@ -11,7 +11,10 @@
import unittest
import mock
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
from mock import Mock
@@ -69,7 +72,7 @@
def testMarkDoomedProjects(self):
self.setUpMarkDoomedProjects()
- reaper = reap.Reap('req', 'resp', services=self.services)
+ reaper = reap.Reap(services=self.services)
self.mox.ReplayAll()
doomed_project_ids = reaper._MarkDoomedProjects(self.cnxn)
@@ -92,7 +95,7 @@
def testExpungeDeletableProjects(self):
self.setUpExpungeParts()
- reaper = reap.Reap('req', 'resp', services=self.services)
+ reaper = reap.Reap(services=self.services)
self.mox.ReplayAll()
expunged_project_ids = reaper._ExpungeDeletableProjects(self.cnxn)
diff --git a/framework/test/sorting_test.py b/framework/test/sorting_test.py
index 4b1feb3..4251308 100644
--- a/framework/test/sorting_test.py
+++ b/framework/test/sorting_test.py
@@ -12,7 +12,10 @@
# For convenient debugging
import logging
-import mox
+try:
+ from mox3 import mox
+except ImportError:
+ import mox
from framework import sorting
from framework import framework_views
diff --git a/framework/test/warmup_test.py b/framework/test/warmup_test.py
index 8140fc7..13223f1 100644
--- a/framework/test/warmup_test.py
+++ b/framework/test/warmup_test.py
@@ -10,26 +10,39 @@
import unittest
-from testing import testing_helpers
+import flask
-from framework import sql
from framework import warmup
-from services import service_manager
class WarmupTest(unittest.TestCase):
- def setUp(self):
- #self.cache_manager = cachemanager_svc.CacheManager()
- #self.services = service_manager.Services(
- # cache_manager=self.cache_manager)
- self.services = service_manager.Services()
- self.servlet = warmup.Warmup('req', 'res', services=self.services)
+ def testHandleWarmup(self):
+ app = flask.Flask(__name__)
+ app.add_url_rule('/', view_func=warmup.Warmup)
+ with app.test_client() as client:
+ response = client.get('/')
- def testHandleRequest_NothingToDo(self):
- mr = testing_helpers.MakeMonorailRequest()
- actual_json_data = self.servlet.HandleRequest(mr)
- self.assertEqual(
- {'success': 1},
- actual_json_data)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.data, '')
+
+ def testHandleStart(self):
+ app = flask.Flask(__name__)
+ app.add_url_rule('/', view_func=warmup.Start)
+
+ with app.test_client() as client:
+ response = client.get('/')
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.data, '')
+
+ def testHandleStop(self):
+ app = flask.Flask(__name__)
+ app.add_url_rule('/', view_func=warmup.Stop)
+
+ with app.test_client() as client:
+ response = client.get('/')
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.data, '')
diff --git a/framework/trimvisitedpages.py b/framework/trimvisitedpages.py
index f036c10..f43ab09 100644
--- a/framework/trimvisitedpages.py
+++ b/framework/trimvisitedpages.py
@@ -11,8 +11,7 @@
from framework import jsonfeed
-# TODO: change to FlaskInternalTask when convert to Flask
-class TrimVisitedPages(jsonfeed.InternalTask):
+class TrimVisitedPages(jsonfeed.FlaskInternalTask):
"""Look for users with more than 10 visited hotlists and deletes extras."""
@@ -20,8 +19,5 @@
"""Delete old RecentHotlist2User rows when there are too many"""
self.services.user.TrimUserVisitedHotlists(mr.cnxn)
- # def GetTrimVisitedPages(self, **kwargs):
- # return self.handler(**kwargs)
-
- # def PostTrimVisitedPages(self, **kwargs):
- # return self.handler(**kwargs)
+ def GetTrimVisitedPages(self, **kwargs):
+ return self.handler(**kwargs)
diff --git a/framework/warmup.py b/framework/warmup.py
index ace76ce..a133107 100644
--- a/framework/warmup.py
+++ b/framework/warmup.py
@@ -10,55 +10,26 @@
import logging
-from framework import jsonfeed
-
-# TODO(https://crbug.com/monorail/6511): Convert to FlaskInternalTask
-class Warmup(jsonfeed.InternalTask):
+def Warmup():
"""Placeholder for warmup work. Used only to enable min_idle_instances."""
-
- def HandleRequest(self, _mr):
- """Don't do anything that could cause a jam when many instances start."""
- logging.info('/_ah/startup does nothing in Monorail.')
- logging.info('However it is needed for min_idle_instances in app.yaml.')
-
- return {
- 'success': 1,
- }
-
- # def GetWarmup(self, **kwargs):
- # return self.handler(**kwargs)
+ # Don't do anything that could cause a jam when many instances start.
+ logging.info('/_ah/startup does nothing in Monorail.')
+ logging.info('However it is needed for min_idle_instances in app.yaml.')
+ return ''
-# TODO(https://crbug.com/monorail/6511): Convert to FlaskInternalTask
-class Start(jsonfeed.InternalTask):
+def Start():
"""Placeholder for start work. Used only to enable manual_scaling."""
-
- def HandleRequest(self, _mr):
- """Don't do anything that could cause a jam when many instances start."""
- logging.info('/_ah/start does nothing in Monorail.')
- logging.info('However it is needed for manual_scaling in app.yaml.')
-
- return {
- 'success': 1,
- }
-
- # def GetStart(self, **kwargs):
- # return self.handler(**kwargs)
+ # Don't do anything that could cause a jam when many instances start.
+ logging.info('/_ah/start does nothing in Monorail.')
+ logging.info('However it is needed for manual_scaling in app.yaml.')
+ return ''
-# TODO(https://crbug.com/monorail/6511): Convert to FlaskInternalTask
-class Stop(jsonfeed.InternalTask):
+def Stop():
"""Placeholder for stop work. Used only to enable manual_scaling."""
-
- def HandleRequest(self, _mr):
- """Don't do anything that could cause a jam when many instances start."""
- logging.info('/_ah/stop does nothing in Monorail.')
- logging.info('However it is needed for manual_scaling in app.yaml.')
-
- return {
- 'success': 1,
- }
-
- # def GetStop(self, **kwargs):
- # return self.handler(**kwargs)
+ # Don't do anything that could cause a jam when many instances start."""
+ logging.info('/_ah/stop does nothing in Monorail.')
+ logging.info('However it is needed for manual_scaling in app.yaml.')
+ return ''