opendev/storyboard
Code Issues Proposed changes

Datetime instances from SQLAlchemy now all contain timezone.

Browse Source 
One of the complex problems in storyboard has been that none of our
datetime instances contain a timezone, making comparison operations
guesswork at best. This is partially exacerbated by oslo.db's
TimestampMixin, which ensures created_at and updated_at parameters
are set, but does not inform the instance that its underlying
assumption is to store datetime in UTC.

This patch addresses this, by creating our own UTCDateTime SQLAlchemy
type decorator and applying it everywhere a datetime is needed.

Change-Id: Iea89661e4d370f0e8af93d2c033b4c44723a39a4
This commit is contained in:
Michael Krotscheck 2015-02-11 13:29:38 -08:00 committed by Aleksey Ripinen
parent 552f2848b3
commit db53c6b2c5
11 changed files with 94 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
storyboard/api/auth/oauth_validator.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
import datetime
import pytz
from oauthlib.oauth2 import RequestValidator
from oauthlib.oauth2 import WebApplicationServer
@ -111,7 +112,7 @@ class SkeletonValidator(RequestValidator):
email = request._params["openid.sreg.email"]
full_name = request._params["openid.sreg.fullname"]
username = request._params["openid.sreg.nickname"]
last_login = datetime.datetime.utcnow()
last_login = datetime.datetime.now(pytz.utc)
user = user_api.user_get_by_openid(openid)
user_dict = {"full_name": full_name,
@ -159,10 +160,10 @@ class SkeletonValidator(RequestValidator):
# yolanda.robla: TEMPORARILY COMMENT THAT UNTIL FIX IS ON PLACE
# Calculate the expiration date.
#expires_on = db_code.created_at + datetime.timedelta(
# seconds=db_code.expires_in)
# expires_on = db_code.created_at + datetime.timedelta(
# seconds=db_code.expires_in)
#
#return expires_on > datetime.datetime.now()
# return expires_on > datetime.datetime.now()
def confirm_redirect_uri(self, client_id, code, redirect_uri, client,
*args, **kwargs):
@ -212,7 +213,7 @@ class SkeletonValidator(RequestValidator):
access_token_values = {
"access_token": token["access_token"],
"expires_in": token["expires_in"],
"expires_at": datetime.datetime.utcnow() + datetime.timedelta(
"expires_at": datetime.datetime.now(pytz.utc) + datetime.timedelta(
seconds=token["expires_in"]),
"user_id": user_id
}
@ -227,7 +228,7 @@ class SkeletonValidator(RequestValidator):
"refresh_token": token["refresh_token"],
"user_id": user_id,
"expires_in": refresh_expires_in,
"expires_at": datetime.datetime.utcnow() + datetime.timedelta(
"expires_at": datetime.datetime.now(pytz.utc) + datetime.timedelta(
seconds=refresh_expires_in),
}
auth_api.refresh_token_save(refresh_token_values)
@ -271,7 +272,7 @@ class SkeletonValidator(RequestValidator):
if not refresh_token_entry:
return False
if datetime.datetime.utcnow() > refresh_token_entry.expires_at:
if datetime.datetime.now(pytz.utc) > refresh_token_entry.expires_at:
auth_api.refresh_token_delete(refresh_token)
return False

9
storyboard/db/api/access_tokens.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
import datetime
import pytz
from storyboard.db.api import base as api_base
from storyboard.db import models
@ -37,7 +38,7 @@ def is_valid(access_token):
if not token:
return False
if datetime.datetime.utcnow() > token.expires_at:
if datetime.datetime.now(tz=pytz.utc) > token.expires_at:
access_token_delete(token.id)
return False
@ -75,9 +76,9 @@ def access_token_get_count(**kwargs):
def access_token_create(values):
# Update the expires_at date.
values['created_at'] = datetime.datetime.utcnow()
values['expires_at'] = datetime.datetime.utcnow() + datetime.timedelta(
seconds=values['expires_in'])
values['created_at'] = datetime.datetime.now(pytz.utc)
values['expires_at'] = datetime.datetime.now(pytz.utc) + datetime \
.timedelta(seconds=values['expires_in'])
return api_base.entity_create(models.AccessToken, values)

41
storyboard/db/decorators.py Normal file
View File

@ -0,0 +1,41 @@
# Copyright 2015 Hewlett-Packard Development Company, L.P.
#
# 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.
from sqlalchemy.types import TypeDecorator, DateTime
import pytz
class UTCDateTime(TypeDecorator):
"""This decorator ensures that timezones will always remain attached to
datetime fields that are written to/from the database, since mysql does
NOT support timezones in its datetime fields. In the case that a
value is provided without a timezone, it will raise an exception to draw
attention to itself: Engineers should always work with timezoned
datetime instances.
"""
impl = DateTime
def process_bind_param(self, value, engine):
if value is not None:
# If the value doesn't have a timezone, raise an exception.
if not value.tzinfo:
raise RuntimeError("Datetime value without timezone provided,"
" please provide a timezone.")
# Convert to UTC, then scrub the timezone for storing in MySQL.
return value.astimezone(pytz.utc).replace(tzinfo=None)
def process_result_value(self, value, engine):
if value is not None:
return value.replace(tzinfo=pytz.utc)

28
storyboard/db/models.py
View File

@ -17,11 +17,15 @@
SQLAlchemy Models for storing storyboard
"""
import datetime
import pytz
import six
import six.moves.urllib.parse as urlparse
from oslo.config import cfg
from oslo.db.sqlalchemy import models
from sqlalchemy import Boolean
from sqlalchemy import Column
from sqlalchemy import DateTime
from sqlalchemy.dialects.mysql import MEDIUMTEXT
from sqlalchemy import Enum
from sqlalchemy.ext import declarative
@ -38,8 +42,7 @@ from sqlalchemy import Unicode
from sqlalchemy import UnicodeText
from sqlalchemy_fulltext import FullText
import six
import six.moves.urllib.parse as urlparse
from storyboard.db.decorators import UTCDateTime
CONF = cfg.CONF
@ -71,7 +74,18 @@ class IdMixin(object):
id = Column(Integer, primary_key=True)
class StoriesBase(models.TimestampMixin,
class UTCTimestampMixin(object):
"""A Database model mixin that automatically manages our creation and
updating timestamps. This mixin was copied from oslo.db, and adapted to
use our own internal UTCDateTime type decorator.
"""
created_at = Column(UTCDateTime,
default=lambda: datetime.datetime.now(tz=pytz.utc))
updated_at = Column(UTCDateTime,
onupdate=lambda: datetime.datetime.now(tz=pytz.utc))
class StoriesBase(UTCTimestampMixin,
IdMixin,
models.ModelBase):
metadata = None
@ -134,7 +148,7 @@ class User(FullText, ModelBuilder, Base):
is_staff = Column(Boolean, default=False)
is_active = Column(Boolean, default=True)
is_superuser = Column(Boolean, default=False)
last_login = Column(DateTime)
last_login = Column(UTCDateTime)
teams = relationship("Team", secondary="team_membership")
permissions = relationship("Permission", secondary="user_permissions")
enable_login = Column(Boolean, default=True)
@ -316,7 +330,7 @@ class AccessToken(ModelBuilder, Base):
access_token = Column(Unicode(CommonLength.top_middle_length),
nullable=False)
expires_in = Column(Integer, nullable=False)
expires_at = Column(DateTime, nullable=False)
expires_at = Column(UTCDateTime, nullable=False)
refresh_tokens = relationship("RefreshToken",
cascade="save-update, merge, delete",
passive_updates=False,
@ -331,7 +345,7 @@ class RefreshToken(ModelBuilder, Base):
refresh_token = Column(Unicode(CommonLength.top_middle_length),
nullable=False)
expires_in = Column(Integer, nullable=False)
expires_at = Column(DateTime, nullable=False)
expires_at = Column(UTCDateTime, nullable=False)
def _story_build_summary_query():

7
storyboard/migrate/launchpad/writer.py
View File

@ -150,12 +150,12 @@ class LaunchpadWriter(object):
"""
if hasattr(bug, 'date_created'):
created_at = bug.date_created.strftime('%Y-%m-%d %H:%M:%S')
created_at = bug.date_created
else:
created_at = None
if hasattr(bug, 'date_last_updated'):
updated_at = bug.date_last_updated.strftime('%Y-%m-%d %H:%M:%S')
updated_at = bug.date_last_updated
else:
updated_at = None
@ -263,8 +263,7 @@ class LaunchpadWriter(object):
for i in range(current_count, desired_count):
print '- Importing comment %s of %s' % (i + 1, desired_count)
message = bug.messages[i]
message_created_at = message.date_created \
.strftime('%Y-%m-%d %H:%M:%S')
message_created_at = message.date_created
message_owner = self.write_user(message.owner)
comment = comments_api.comment_create({

2
storyboard/plugin/cron/base.py
View File

@ -109,7 +109,7 @@ class CronPluginBase(plugin_base.PluginBase):
lr_file = os.path.join(cron_directory, plugin_name)
now = pytz.utc.localize(datetime.datetime.utcnow())
now = datetime.datetime.now(pytz.utc)
start_time = self._get_file_mtime(path=lr_file)
end_time = self._get_file_mtime(path=lr_file,

3
storyboard/plugin/token_cleaner/cleaner.py
View File

@ -14,6 +14,7 @@
from datetime import datetime
from datetime import timedelta
import pytz
import storyboard.db.api.base as api_base
from storyboard.db.models import AccessToken
@ -49,7 +50,7 @@ class TokenCleaner(CronPluginBase):
:param end_time: The current timestamp.
"""
# Calculate last week.
lastweek = datetime.utcnow() - timedelta(weeks=1)
lastweek = datetime.now(pytz.utc) - timedelta(weeks=1)
# Build the query.
query = api_base.model_query(AccessToken.id)

3
storyboard/tests/db/api/test_access_tokens.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
from datetime import datetime
import pytz
from storyboard.db.api import access_tokens
from storyboard.db.api import users
@ -29,7 +30,7 @@ class TokenTest(base.BaseDbTestCase):
"access_token": u'an_access_token',
"refresh_token": u'a_refresh_token',
"expires_in": 3600,
"expires_at": datetime.utcnow(),
"expires_at": datetime.now(pytz.utc),
"user_id": 1
}

11
storyboard/tests/mock_data.py
View File

@ -13,6 +13,7 @@
# under the License.
import datetime
import pytz
import storyboard.common.event_types as event
from storyboard.db.api import base as db
@ -31,7 +32,7 @@ def load():
"""Load a batch of useful data into the database that our tests can work
with.
"""
now = datetime.datetime.utcnow()
now = datetime.datetime.now(tz=pytz.utc)
expires_at = now + datetime.timedelta(seconds=3600)
expired_at = now + datetime.timedelta(seconds=-3600)
@ -63,22 +64,22 @@ def load():
user_id=1,
access_token='valid_superuser_token',
expires_in=3600,
expires_at=expires_at.strftime('%Y-%m-%d %H:%M:%S')),
expires_at=expires_at),
AccessToken(
user_id=1,
access_token='expired_superuser_token',
expires_in=3600,
expires_at=expired_at.strftime('%Y-%m-%d %H:%M:%S')),
expires_at=expired_at),
AccessToken(
user_id=2,
access_token='valid_user_token',
expires_in=3600,
expires_at=expires_at.strftime('%Y-%m-%d %H:%M:%S')),
expires_at=expires_at),
AccessToken(
user_id=2,
access_token='expired_user_token',
expires_in=3600,
expires_at=expired_at.strftime('%Y-%m-%d %H:%M:%S'))
expires_at=expired_at)
])
# Create some test projects.

3
storyboard/tests/plugin/cron/test_base.py
View File

@ -105,8 +105,7 @@ class TestCronPluginBase(base.WorkingDirTestCase):
# Current timestamp, remove microseconds so that we don't run into
# execution time delay problems.
now = pytz.utc.localize(datetime.datetime.utcnow()) \
.replace(microsecond=0)
now = datetime.datetime.now(pytz.utc).replace(microsecond=0)
# Check the plugin's params.
self.assertEqual(last_run_date, plugin.last_invocation_parameters[0])

7
storyboard/tests/plugin/token_cleaner/test_cleaner.py
View File

@ -14,6 +14,7 @@
from datetime import datetime
from datetime import timedelta
import pytz
from oslo.config import cfg
import storyboard.db.api.base as db_api
@ -67,16 +68,16 @@ class TestTokenCleaner(db_base.BaseDbTestCase,
new_access_tokens = []
new_refresh_tokens = []
for i in range(0, 100):
created_at = datetime.utcnow() - timedelta(days=i)
created_at = datetime.now(pytz.utc) - timedelta(days=i)
expires_in = (60 * 60 * 24) - 5 # Minus five seconds, see above.
expires_at = created_at + timedelta(seconds=expires_in)
new_access_tokens.append(
AccessToken(
user_id=1,
created_at=created_at.strftime('%Y-%m-%d %H:%M:%S'),
created_at=created_at,
expires_in=expires_in,
expires_at=expires_at.strftime('%Y-%m-%d %H:%M:%S'),
expires_at=expires_at,
access_token='test_token_%s' % (i,))
)
new_access_tokens = load_data(new_access_tokens)