Changeset d6f8304 in OpenWorkouts-current


Ignore:
Timestamp:
Feb 11, 2019, 6:55:55 PM (5 years ago)
Author:
Borja Lopez <borja@…>
Branches:
current, feature/docs, master
Children:
02aee97
Parents:
93bbb89
Message:

(#52) - screenshots of the workouts maps were corrupted randomly.

Replaced our screenshot_map shell script that was calling chrome headless
directly with some python code running splinter + selenium webdriver.

Using chromedriver in such environment we can first visit the map site
for the given workout, then wait a bit for it to completely load, then
take the screenshot.

I've also removed all traces of screenshot_map from the code.

Files:
1 deleted
8 edited

Legend:

Unmodified
Added
Removed

  • bin/install

    r93bbb89 rd6f8304  
    1616    chmod u+x ${current}/bin/js_deps
    1717    chmod u+x ${current}/bin/start
    18     chmod u+x ${current}/bin/screenshot_map
    1918}
    2019

  • ow/models/workout.py

    r93bbb89 rd6f8304  
    423423        return self.fit_file is not None
    424424
    425     @property
    426     def map_screenshot(self):
     425    def map_screenshot(self, request):
    427426        """
    428427        Return the static path to the screenshot image of the map for
    … …  
    432431            return None
    433432
     433        screenshot_name = os.path.join(
     434            str(self.owner.uid), str(self.workout_id) + '.png')
    434435        current_path = os.path.abspath(os.path.dirname(__file__))
    435436        screenshot_path = os.path.join(
    436             current_path, '../static/maps',
    437             str(self.owner.uid), str(self.workout_id)) + '.png'
     437            current_path, '../static/maps', screenshot_name)
    438438
    439439        if not os.path.exists(screenshot_path):
    440440            # screenshot does not exist, generate it
    441             save_map_screenshot(self)
    442 
    443         # the value returned is relative to the static files served
    444         # by the app, so we can use request.static_url() with it
    445         static_path = os.path.join('static/maps', str(self.owner.uid),
    446                                    str(self.workout_id))
    447         return 'ow:' + static_path + '.png'
     441            save_map_screenshot(self, request)
     442
     443        static_path = os.path.join('static/maps', screenshot_name)
     444        return request.static_url('ow:' + static_path)

  • ow/templates/dashboard.pt

    r93bbb89 rd6f8304  
    8080            <div class="workout-map" tal:condition="workout.has_gpx">
    8181                <a href="" tal:attributes="href request.resource_url(workout)">
    82                     <img src="" tal:attributes="src request.static_url(workout.map_screenshot);
     82                    <img src="" tal:attributes="src workout.map_screenshot(request);
    8383                              alt workout.title; title workout.title">
    8484                </a>

  • ow/templates/profile.pt

    r93bbb89 rd6f8304  
    144144              <div class="workout-map" tal:condition="workout.has_gpx">
    145145                <a href="" tal:attributes="href request.resource_url(workout)">
    146                   <img src="" tal:attributes="src request.static_url(workout.map_screenshot);
     146                  <img src="" tal:attributes="src workout.map_screenshot(request);
    147147                            alt workout.title; title workout.title">
    148148                </a>

  • ow/tests/models/test_workout.py

    r93bbb89 rd6f8304  
    77import pytest
    88from pyramid.security import Allow, Everyone, Deny, ALL_PERMISSIONS
     9from pyramid.testing import DummyRequest
    910
    1011from ow.models.workout import Workout
    … …  
    576577    def test_map_screenshot_no_gpx(self, sms, os, root):
    577578        workout = root['john']['1']
    578         assert workout.map_screenshot is None
     579        request = DummyRequest()
     580        assert workout.map_screenshot(request) is None
    579581        assert not os.path.abspath.called
    580582        assert not os.path.dirname.called
    … …  
    602604
    603605        uid = str(root['john'].uid)
    604         assert workout.map_screenshot == 'ow:/static/maps/' + uid + '/1.png'
     606        request = DummyRequest()
     607        # dummyrequest can't resolve static assets without adding a lot
     608        # of boilerplate, no need for that here
     609        request.static_url = Mock()
     610        request.static_url.return_value = 'ow:/static/maps/' + uid + '/1.png'
     611        res = workout.map_screenshot(request)
     612        assert res == 'ow:/static/maps/' + uid + '/1.png'
    605613        assert os.path.abspath.called
    606614        assert os.path.dirname.called
    607         assert os.path.join.call_count == 2
     615        assert os.path.join.call_count == 3
    608616        assert os.path.exists.called
    609         sms.assert_called_once_with(workout)
     617        sms.assert_called_once_with(workout, request)
    610618
    611619    @patch('ow.models.workout.os')
    … …  
    619627        os.path.join.side_effect = join
    620628        # This forces the "save screenshot" code NOT to be run
    621         os.path.exists.return_value = True
     629        os.path.eisxts.return_value = True
    622630
    623631        workout = root['john']['1']
    … …  
    626634
    627635        uid = str(root['john'].uid)
    628         assert workout.map_screenshot == 'ow:/static/maps/' + uid + '/1.png'
     636        request = DummyRequest()
     637        # dummyrequest can't resolve static assets without adding a lot
     638        # of boilerplate, no need for that here
     639        request.static_url = Mock()
     640        request.static_url.return_value = 'ow:/static/maps/' + uid + '/1.png'
     641        res = workout.map_screenshot(request)
     642        assert res == 'ow:/static/maps/' + uid + '/1.png'
    629643        assert os.path.abspath.called
    630644        assert os.path.dirname.called
    631         assert os.path.join.call_count == 2
     645        assert os.path.join.call_count == 3
    632646        assert os.path.exists.called
    633647        assert not sms.called

  • ow/tests/test_utilities.py

    r93bbb89 rd6f8304  
    11import os
    22from datetime import timedelta, datetime
    3 from unittest.mock import patch
     3from unittest.mock import patch, Mock
    44from pyexpat import ExpatError
    55from xml.dom.minidom import Element
    66
    77import pytest
     8from pyramid.testing import DummyRequest
    89
    910from ow.models.root import OpenWorkouts
    … …  
    8384        assert kmph_to_mps(30) == 30 * 0.277778
    8485
     86    @patch('ow.utilities.shutil')
    8587    @patch('ow.utilities.os')
    86     @patch('ow.utilities.subprocess')
    87     def test_save_map_screenshot_no_gpx(self, subprocess, os, root, john):
    88         saved = save_map_screenshot(john['1'])
     88    @patch('ow.utilities.Browser')
     89    def test_save_map_screenshot_no_gpx(
     90            self, Browser, os, shutil, root, john):
     91        request = DummyRequest()
     92        saved = save_map_screenshot(john['1'], request)
    8993        assert not saved
     94        assert not Browser.called
    9095        assert not os.path.abspath.called
    9196        assert not os.path.dirname.called
    … …  
    9398        assert not os.path.exists.called
    9499        assert not os.makedirs.called
    95         assert not subprocess.run.called
     100        assert not shutil.move.called
    96101        # even having a fit tracking file, nothing is done
    97102        john['1'].tracking_file = 'faked fit file'
    98103        john['1'].tracking_filetype = 'fit'
    99         saved = save_map_screenshot(john['1'])
     104        saved = save_map_screenshot(john['1'], request)
    100105        assert not saved
     106        assert not Browser.called
    101107        assert not os.path.abspath.called
    102108        assert not os.path.dirname.called
    … …  
    104110        assert not os.path.exists.called
    105111        assert not os.makedirs.called
    106         assert not subprocess.run.called
    107 
     112        assert not shutil.move.called
     113
     114    @patch('ow.utilities.shutil')
    108115    @patch('ow.utilities.os')
    109     @patch('ow.utilities.subprocess')
    110     def test_save_map_screenshot_with_gpx(self, subprocess, os, root, john):
     116    @patch('ow.utilities.Browser')
     117    def test_save_map_screenshot_with_gpx(
     118            self, Browser, os, shutil, root, john):
     119        request = DummyRequest()
     120        browser = Mock()
     121        Browser.return_value = browser
    111122        os.path.abspath.return_value = 'current_dir'
    112123        os.path.join.side_effect = join
    … …  
    116127        os.path.exists.return_value = False
    117128
     129        map_url = request.resource_url(john['1'], 'map')
     130
    118131        john['1'].tracking_file = 'faked gpx content'
    119132        john['1'].tracking_filetype = 'gpx'
    120         saved = save_map_screenshot(john['1'])
     133        saved = save_map_screenshot(john['1'], request)
    121134        assert saved
     135        Browser.assert_called_once_with('chrome', headless=True)
     136        browser.driver.set_window_size.assert_called_once_with(1300, 436)
     137        browser.visit.assert_called_once_with(map_url)
     138        browser.screenshot.assert_called_once
    122139        os.path.abspath.assert_called_once
    123140        assert os.path.dirname.called
    124         assert os.path.join.call_count == 3
     141        assert os.path.join.call_count == 2
    125142        assert os.path.exists.called
    126143        assert os.makedirs.called
    127         subprocess.run.assert_called_once
    128 
     144        os.shutil.move.assert_called_once
     145
     146    @patch('ow.utilities.shutil')
    129147    @patch('ow.utilities.os')
    130     @patch('ow.utilities.subprocess')
     148    @patch('ow.utilities.Browser')
    131149    def test_save_map_screenshot_with_gpx_makedirs(
    132             self, subprocess, os, root, john):
     150            self, Browser, os, shutil, root, john):
     151        request = DummyRequest()
     152        browser = Mock()
     153        Browser.return_value = browser
    133154        os.path.abspath.return_value = 'current_dir'
    134155        os.path.join.side_effect = join
    … …  
    136157        os.path.exists.return_value = True
    137158
     159        map_url = request.resource_url(john['1'], 'map')
     160
    138161        john['1'].tracking_file = 'faked gpx content'
    139162        john['1'].tracking_filetype = 'gpx'
    140         saved = save_map_screenshot(john['1'])
     163        saved = save_map_screenshot(john['1'], request)
    141164        assert saved
     165        Browser.assert_called_once_with('chrome', headless=True)
     166        browser.driver.set_window_size.assert_called_once_with(1300, 436)
     167        browser.visit.assert_called_once_with(map_url)
     168        browser.screenshot.assert_called_once
    142169        os.path.abspath.assert_called_once
    143170        assert os.path.dirname.called
    144         assert os.path.join.call_count == 3
     171        assert os.path.join.call_count == 2
    145172        assert os.path.exists.called
    146173        assert not os.makedirs.called
    147         subprocess.run.assert_called_once
     174        os.shutil.move.assert_called_once
    148175
    149176    def test_timedelta_to_hms(self):

  • ow/utilities.py

    r93bbb89 rd6f8304  
    33import logging
    44import calendar
    5 import subprocess
     5import shutil
     6import time
    67from datetime import datetime, timedelta
    78from decimal import Decimal
    … …  
    1112from xml.dom import minidom
    1213from ZODB.blob import Blob
     14from splinter import Browser
     15
    1316
    1417log = logging.getLogger(__name__)
    … …  
    192195
    193196
    194 def save_map_screenshot(workout):
     197def save_map_screenshot(workout, request):
     198
    195199    if workout.has_gpx:
     200
     201        map_url = request.resource_url(workout, 'map')
     202
     203        browser = Browser('chrome', headless=True)
     204        browser.driver.set_window_size(1300, 436)
     205
     206        browser.visit(map_url)
     207        # we need to wait a moment before taking the screenshot, to ensure
     208        # all tiles are loaded in the map.
     209        time.sleep(5)
     210
     211        # splinter saves the screenshot with a random name (even if we do
     212        # provide a name) so we get the path to that file and later on we
     213        # move it to the proper place
     214        splinter_screenshot_path = browser.screenshot()
     215
    196216        current_path = os.path.abspath(os.path.dirname(__file__))
    197         tool_path = os.path.join(current_path, '../bin/screenshot_map')
    198 
    199217        screenshots_path = os.path.join(
    200218            current_path, 'static/maps', str(workout.owner.uid))
    … …  
    206224        screenshot_path += '.png'
    207225
    208         subprocess.run(
    209             [tool_path, str(workout.owner.uid), str(workout.workout_id),
    210              screenshot_path], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     226        shutil.move(splinter_screenshot_path, screenshot_path)
    211227
    212228        return True

  • setup.py

    r93bbb89 rd6f8304  
    3030    'lxml',
    3131    'pytz',
    32     'fitparse'
     32    'fitparse',
     33    'splinter'
    3334]
    3435
Note: See TracChangeset for help on using the changeset viewer.