From 1f3816e452578e5462c63bdd0e4fdbf581e353dc Mon Sep 17 00:00:00 2001 From: Stefan Siegel Date: Fri, 20 Nov 2015 00:46:52 +0100 Subject: [PATCH 1/2] Remove Selenium dependency The new code uses the (internal) BGG JSON API directly. This removes many dependencies, speeds up the process, makes it more reliable and makes it possible to handle several copies of a game at the same time. When "collid" is empty or --force-new is given, the importer creates new copies of the games. Without --force-new and with non-empty "collid" matching existing copies of the games will be updated. --- README.rst | 26 +-- bggcli/__init__.py | 6 +- bggcli/commands/collection_delete.py | 48 +++-- bggcli/commands/collection_export.py | 55 +++--- bggcli/commands/collection_import.py | 119 +++++++++-- bggcli/main.py | 24 +-- bggcli/ui/__init__.py | 51 ----- bggcli/ui/collectionpage.py | 26 --- bggcli/ui/gamepage.py | 282 --------------------------- bggcli/ui/loginpage.py | 53 ----- bggcli/util/csvreader.py | 11 +- bggcli/util/webdriver.py | 80 -------- bggcli/util/xmltocsv.py | 13 ++ setup.py | 2 +- tests/commons.py | 2 +- tests/resources/collection.csv | 6 +- tests/test_collection.py | 53 +++-- 17 files changed, 229 insertions(+), 628 deletions(-) delete mode 100644 bggcli/ui/__init__.py delete mode 100644 bggcli/ui/collectionpage.py delete mode 100644 bggcli/ui/gamepage.py delete mode 100644 bggcli/ui/loginpage.py delete mode 100644 bggcli/util/webdriver.py diff --git a/README.rst b/README.rst index ed139ca..9afe78e 100644 --- a/README.rst +++ b/README.rst @@ -44,9 +44,6 @@ Python 2.7 is required. Usage ===== -You'll need **Firefox** to be installed; Firefox will be automatically controlled by ``bggcli`` to perform operations -(through Selenium library). - Type ``bggcli`` to get the full help about available commands. Here is an example to export a collection from an account *account1* and import it to another account *account2*:: @@ -75,8 +72,9 @@ Notes: * Column names are those exported by BGG. Any column not recognized will just be ignored * When a game already exists in your collection, game is updated with provided CSV values only, other fields are not impacted. You could only update one field for all your game. - * Games are identified by their internal ID, named ``objectid`` in CSV file (name used by BGG). Having the - ``objectname`` field (name of the game) is also recommended for logging. + * Games are identified by their internal ID, named ``objectid`` in CSV file (name used by BGG). Collection items are + identified by the ID named ``collid`` (for modifying or deleting existing items). Having the ``objectname`` field + (name of the game) is also recommended for logging. Remove games from a collection @@ -89,7 +87,7 @@ Example::: Notes: - * Only the ``objectid`` column will be used for this operation: this is the internal ID managed by BGG. All other + * Only the ``collid`` column will be used for this operation: this is the internal ID managed by BGG. All other columns will just be ignored. Export a collection @@ -102,22 +100,16 @@ Example::: Notes: - * Only the ``objectid`` column will be used for this operation: this is the internal ID managed by BGG. All other + * Only the ``collid`` column will be used for this operation: this is the internal ID managed by BGG. All other columns will just be ignored. Limitations =========== -* Only *Firefox* is supported. This tools relies on Selenium to control browser, and only Firefox is supported - out of the box by Selenium (i.e. without additional requirements). Support of additional browsers could be introduced, - but I'm not sure it's worth it. -* Performance: Selenium+Firefox association is not the fastest way to automate operations, but it's - probably the best regarding stability (no Javascript emulation, Firefox does the work) and simplicity (no need to - install anything else), which is the most important in the current context. On my laptop, I see the import taking - 1 min for 5 games. -* Some fields related to the game's version are not exported by BGG: the ``barcode`` and the `language``. Note - although this only applies to custom version you define yourself, which should be quite rare. +* Some fields are not exported by BGG: the ``language`` field related to the game's version and the ``Inventory Date`` + and ``Inventory Location`` in the private info. The missing ``language`` field only applies to custom version you + define yourself, which should be quite rare. Ideas for future versions @@ -140,4 +132,4 @@ Links Final note ========== -Does it really deserve such a development? Probably not, but my second goal was to discover the Python ecosystem! \ No newline at end of file +Does it really deserve such a development? Probably not, but my second goal was to discover the Python ecosystem! diff --git a/bggcli/__init__.py b/bggcli/__init__.py index 4f7e20f..6e46a5f 100644 --- a/bggcli/__init__.py +++ b/bggcli/__init__.py @@ -10,11 +10,13 @@ "changed and bggcli must be updated, or the site is down for " \ "maintenance." -BGG_SUPPORTED_FIELDS = ['objectname', 'objectid', 'rating', 'own', +BGG_SUPPORTED_FIELDS = ['objectname', 'objectid', 'rating', 'numplays', 'own', 'fortrade', 'want', 'wanttobuy', 'wanttoplay', 'prevowned', 'preordered', 'wishlist', 'wishlistpriority', 'wishlistcomment', 'comment', 'conditiontext', 'haspartslist', 'wantpartslist', - 'publisherid', 'imageid', 'year', 'language', 'other', 'pricepaid', + 'collid', 'baverage', 'average', 'numowned', 'objecttype', 'minplayers', + 'maxplayers', 'playingtime', 'maxplaytime', 'minplaytime', 'yearpublished', + 'publisherid', 'imageid', 'year', 'language', 'other', 'barcode', 'pricepaid', 'pp_currency', 'currvalue', 'cv_currency', 'acquisitiondate', 'acquiredfrom', 'quantity', 'privatecomment', '_versionid'] diff --git a/bggcli/commands/collection_delete.py b/bggcli/commands/collection_delete.py index fd639dc..5f07539 100644 --- a/bggcli/commands/collection_delete.py +++ b/bggcli/commands/collection_delete.py @@ -2,7 +2,6 @@ Delete games in your collection from a CSV file. BE CAREFUL, this action is irreversible! Usage: bggcli [-v] -l -p - [-c =]... collection-delete [--force] Options: @@ -10,27 +9,34 @@ -l, --login Your login on BGG -p, --password Your password on BGG --force Force deletion, without any confirmation - -c To specify advanced options, see below - -Advanced options: - browser-keep= If you want to keep your web browser opened at the end of the - operation - browser-profile-dir= Path or your browser profile if you want to use an existing Arguments: The CSV file with games to delete """ +from urllib import urlencode +import cookielib +import urllib2 import sys +from bggcli import BGG_BASE_URL from bggcli.commands import check_file from bggcli.util.csvreader import CsvReader -from bggcli.ui.gamepage import GamePage -from bggcli.ui.loginpage import LoginPage from bggcli.util.logger import Logger -from bggcli.util.webdriver import WebDriver -def execute(args, options): +def game_deleter(opener, row): + collid = row['collid'] + if not collid: + return + + response = opener.open(BGG_BASE_URL + '/geekcollection.php', urlencode({ + 'ajax': 1, 'action': 'delete', 'collid': collid})) + + if response.code != 200: + Logger.error("Failed to delete 'collid'=%s!" % collid, sysexit=True) + + +def execute(args): login = args['--login'] file_path = check_file(args) @@ -53,11 +59,17 @@ def execute(args, options): Logger.info("Deleting games for '%s' account..." % login) - with WebDriver('collection-delete', args, options) as web_driver: - if not LoginPage(web_driver.driver).authenticate(login, args['--password']): - sys.exit(1) + cj = cookielib.CookieJar() + opener = urllib2.build_opener(urllib2.HTTPCookieProcessor(cj)) + Logger.info("Authenticating...", break_line=False) + opener.open(BGG_BASE_URL + '/login', urlencode({ + 'action': 'login', 'username': login, 'password': args['--password']})) + if not any(cookie.name == "bggusername" for cookie in cj): + Logger.info(" [error]", append=True) + Logger.error("Authentication failed for user '%s'!" % login, sysexit=True) + Logger.info(" [done]", append=True) + - Logger.info("Deleting %s games..." % game_count) - game_page = GamePage(web_driver.driver) - csv_reader.iterate(lambda row: game_page.delete(row)) - Logger.info("Deletion has finished.") + Logger.info("Deleting %s games..." % game_count) + csv_reader.iterate(lambda row: game_deleter(opener, row)) + Logger.info("Deletion has finished.") diff --git a/bggcli/commands/collection_export.py b/bggcli/commands/collection_export.py index ef8a2f5..7394027 100644 --- a/bggcli/commands/collection_export.py +++ b/bggcli/commands/collection_export.py @@ -2,77 +2,67 @@ Export a game collection as a CSV file. Usage: bggcli [-v] -l -p - [-c =]... - collection-export + collection-export [--save-xml-file] Options: -v Activate verbose logging -l, --login Your login on BGG -p, --password Your password on BGG - -c To specify advanced options, see below - -Advanced options: - save-xml-file= To store the exported raw XML file in addition (will be + --save-xml-file To store the exported raw XML file in addition (will be save aside the CSV file, with '.xml' extension - browser-keep= If you want to keep your web browser opened at the end of the - operation - browser-profile-dir= Path or your browser profile if you want to use an existing Arguments: The CSV file to generate """ import csv +from urllib import urlencode +import cookielib import urllib2 import time import sys import xml.etree.ElementTree as ET from bggcli import BGG_BASE_URL, BGG_SUPPORTED_FIELDS -from bggcli.ui.loginpage import LoginPage from bggcli.util.logger import Logger -from bggcli.util.webdriver import WebDriver from bggcli.util.xmltocsv import XmlToCsv -BGG_SESSION_COOKIE_NAME = 'SessionID' EXPORT_QUERY_INTERVAL = 5 ERROR_FILE_PATH = 'error.txt' -def execute(args, options): +def execute(args): login = args['--login'] dest_path = args[''] Logger.info("Exporting collection for '%s' account..." % login) # 1. Authentication - with WebDriver('collection-export', args, options) as web_driver: - if not LoginPage(web_driver.driver).authenticate(login, args['--password']): - sys.exit(1) - auth_cookie = web_driver.driver.get_cookie(BGG_SESSION_COOKIE_NAME)['value'] + cj = cookielib.CookieJar() + opener = urllib2.build_opener(urllib2.HTTPCookieProcessor(cj)) + Logger.info("Authenticating...", break_line=False) + opener.open(BGG_BASE_URL + '/login', urlencode({ + 'action': 'login', 'username': login, 'password': args['--password']})) + if not any(cookie.name == "bggusername" for cookie in cj): + Logger.info(" [error]", append=True) + Logger.error("Authentication failed for user '%s'!" % login, sysexit=True) + Logger.info(" [done]", append=True) # 2. Export - # Easier to rely on a client HTTP call rather than Selenium to download a file - # Just need to pass the session cookie to get the full export with private information - # Use XML2 API, see https://www.boardgamegeek.com/wiki/page/BGG_XML_API2#Collection # Default CSV export doesn't provide version info! - url = '%s/xmlapi2/collection?username=%s&version=1&showprivate=1&stats=1' \ - % (BGG_BASE_URL, login) - req = urllib2.Request(url, None, {'Cookie': '%s=%s' % (BGG_SESSION_COOKIE_NAME, auth_cookie)}) + url = BGG_BASE_URL + '/xmlapi2/collection?' + urlencode({ + 'username': login, 'version': 1, 'showprivate': 1, 'stats': 1}) + req = urllib2.Request(url) - # Get a BadStatusLine error most of times without this delay! - # Related to Selenium, but in some conditions that I have not identified - time.sleep(8) try: Logger.info('Launching export...') - response = default_export(req) + response = default_export(opener, req) except Exception as e: Logger.error('Error while fetching export file!', e, sysexit=True) return # 3. Store XML file if requested - xml_file = options.get('save-xml-file') - if xml_file == 'true': + if args['--save-xml-file']: xml_file_path = write_xml_file(response, dest_path) Logger.info("XML file save as %s" % xml_file_path) source = open(xml_file_path, 'rU') @@ -92,13 +82,13 @@ def execute(args, options): Logger.info("Collection has been exported as %s" % dest_path) -def default_export(req): - response = urllib2.urlopen(req) +def default_export(opener, req): + response = opener.open(req) if response.code == 202: Logger.info('Export is queued, will retry in %ss' % EXPORT_QUERY_INTERVAL) time.sleep(EXPORT_QUERY_INTERVAL) - return default_export(req) + return default_export(opener, req) if response.code == 200: return response @@ -135,4 +125,3 @@ def write_csv(source, dest_path): if elem.tag == 'item' and elem.attrib.get('subtype') == 'boardgame': row = XmlToCsv.convert_item(elem) csv_writer.writerow(row) - diff --git a/bggcli/commands/collection_import.py b/bggcli/commands/collection_import.py index cdaaf65..c758444 100644 --- a/bggcli/commands/collection_import.py +++ b/bggcli/commands/collection_import.py @@ -5,33 +5,109 @@ collection. Only the fields defined in the file will be updated. Usage: bggcli [-v] -l -p - [-c =]... - collection-import + collection-import [--force-new] Options: -v Activate verbose logging -l, --login Your login on BGG -p, --password Your password on BGG - -c To specify advanced options, see below - -Advanced options: - browser-keep= If you want to keep your web browser opened at the end of the - operation - browser-profile-dir= Path or your browser profile if you want to use an existing + --force-new Ignore collid, always insert new items Arguments: The CSV file with games to import """ +import re import sys +from urllib import urlencode +import cookielib +import urllib2 + +from bggcli import BGG_BASE_URL from bggcli.commands import check_file -from bggcli.ui.gamepage import GamePage -from bggcli.ui.loginpage import LoginPage from bggcli.util.csvreader import CsvReader from bggcli.util.logger import Logger -from bggcli.util.webdriver import WebDriver -def execute(args, options): +def create_collid(opener, objectid): + response = opener.open(BGG_BASE_URL + '/geekcollection.php', urlencode({ + 'ajax': 1, 'action': 'additem', 'force': 'true', + 'objecttype': 'thing', 'objectid': objectid})) + + if response.code != 200: + Logger.error("Failed to create item of 'objectid'=%s!" % objectid, sysexit=True) + + # There seems to be no straightforward way to get the collid of the item just created. + # To work around this we fetch a list of all items of this objectid and scrape it to + # find the largest collid. This might fail if the collection is concurrently modified. + + response = opener.open(BGG_BASE_URL + '/geekcollection.php?' + urlencode({ + 'ajax': 1, 'action': 'module', 'objecttype': 'thing', 'objectid': objectid})) + + return max(int(m.group(1)) for m in re.finditer( + r"(?i)]*>", response.read())) + + +def update_collid(opener, collid, fieldname, values): + values.update({'ajax': 1, 'action': 'savedata', + 'collid': collid, 'fieldname': fieldname}) + values = { k:unicode(v).encode('utf-8') for k,v in values.iteritems() } + response = opener.open(BGG_BASE_URL + '/geekcollection.php', urlencode(values)) + + if response.code != 200: + Logger.error("Failed to update 'collid'=%s!" % collid, sysexit=True) + + +def game_importer(opener, row, force_new=False): + collid = row['collid'] + + if force_new or not collid: + objectid = row['objectid'] + if not objectid.isdigit(): + Logger.error("Invalid 'objectid'=%s!" % objectid, sysexit=True) + collid = create_collid(opener, objectid) + + values = { k:v for k,v in row.iteritems() if k in [ + 'own', 'prevowned', 'fortrade', 'want', 'wanttobuy', + 'wishlist', 'wishlistpriority', 'wanttoplay', 'preordered'] } + if len(values): + update_collid(opener, collid, 'status', values) + + values = { k:v for k,v in row.iteritems() if k in [ + 'pp_currency', 'pricepaid', 'cv_currency', 'currvalue', 'quantity', + 'acquisitiondate', 'acquiredfrom', 'privatecomment', 'invdate', 'invlocation'] } + if len(values): + update_collid(opener, collid, 'ownership', values) + + if '_versionid' in row.keys() and row['_versionid'].isdigit(): + update_collid(opener, collid, 'version', {'geekitem_version': 1, 'objectid': row['_versionid']}) + elif 'geekitem_version' in row.keys() and 'objectid' in row.keys() and \ + row['geekitem_version'].isdigit() and int(row['geekitem_version']) == 1: + update_collid(opener, collid, 'version', {'geekitem_version': 1, 'objectid': row['objectid']}) + else: + values = { k:v for k,v in row.iteritems() if k in [ + 'imageid', 'publisherid', 'languageid', 'year', 'other', 'barcode'] } + if len(values): + update_collid(opener, collid, 'version', values) + + if 'objectname' in row.keys(): + update_collid(opener, collid, 'objectname', {'value': row['objectname']}) + if 'rating' in row.keys(): + update_collid(opener, collid, 'rating', {'rating': row['rating']}) + if 'weight' in row.keys(): + update_collid(opener, collid, 'weight', {'weight': row['weight']}) + if 'comment' in row.keys(): + update_collid(opener, collid, 'comment', {'value': row['comment']}) + if 'conditiontext' in row.keys(): + update_collid(opener, collid, 'conditiontext', {'value': row['conditiontext']}) + if 'wantpartslist' in row.keys(): + update_collid(opener, collid, 'wantpartslist', {'value': row['wantpartslist']}) + if 'haspartslist' in row.keys(): + update_collid(opener, collid, 'haspartslist', {'value': row['haspartslist']}) + if 'wishlistcomment' in row.keys(): + update_collid(opener, collid, 'wishlistcomment', {'value': row['wishlistcomment']}) + + +def execute(args): login = args['--login'] file_path = check_file(args) @@ -41,11 +117,16 @@ def execute(args, options): Logger.info("Importing games for '%s' account..." % login) - with WebDriver('collection-import', args, options) as web_driver: - if not LoginPage(web_driver.driver).authenticate(login, args['--password']): - sys.exit(1) + cj = cookielib.CookieJar() + opener = urllib2.build_opener(urllib2.HTTPCookieProcessor(cj)) + Logger.info("Authenticating...", break_line=False) + opener.open(BGG_BASE_URL + '/login', urlencode({ + 'action': 'login', 'username': login, 'password': args['--password']})) + if not any(cookie.name == "bggusername" for cookie in cj): + Logger.info(" [error]", append=True) + Logger.error("Authentication failed for user '%s'!" % login, sysexit=True) + Logger.info(" [done]", append=True) - Logger.info("Importing %s games..." % csv_reader.rowCount) - game_page = GamePage(web_driver.driver) - csv_reader.iterate(lambda row: game_page.update(row)) - Logger.info("Import has finished.") + Logger.info("Importing %s games..." % csv_reader.rowCount) + csv_reader.iterate(lambda row: game_importer(opener, row, args['--force-new'])) + Logger.info("Import has finished.") diff --git a/bggcli/main.py b/bggcli/main.py index d11823c..0bc40cf 100644 --- a/bggcli/main.py +++ b/bggcli/main.py @@ -3,7 +3,6 @@ Command Line Interface for BoardGameGeek.com Usage: bggcli [--version] [-v] [-l ] [-p ] - [-c =]... [...] Options: @@ -11,13 +10,6 @@ -v Activate verbose logging -l, --login Your login on BGG -p, --password Your password on BGG - -c To specify advanced options, see below - -Advanced options: - browser-keep= If you want to keep your web browser opened at the end of the - operation - browser-profile-dir= Path or your browser profile if you want to use an existing - profile (useful for debugging purpose) Available commands are: help Display general help or help for a specific command @@ -30,7 +22,7 @@ import sys import time -from selenium.common.exceptions import WebDriverException +from urllib2 import URLError from docopt import docopt @@ -92,15 +84,9 @@ def _main(argv): def parse_commad_args(command_module, argv): - result = docopt(command_module.__doc__, argv, version='bggcli %s' % VERSION, + return docopt(command_module.__doc__, argv, version='bggcli %s' % VERSION, options_first=False) - try: - return result, explode_dyn_args(result['-c']) - except StandardError: - Logger.info('Invalid syntax for -c option, should be "-c key=value"!') - return None - def show_duration(timer_start): m, s = divmod(time.time() - timer_start, 60) @@ -116,14 +102,14 @@ def execute_command(command, argv): try: command_module = import_command_module(command) - command_args, command_args_options = parse_commad_args(command_module, argv) + command_args = parse_commad_args(command_module, argv) if command_args: - command_module.execute(command_args, command_args_options) + command_module.execute(command_args) show_duration(timer_start) except ImportError: exit_unknown_command(command) - except WebDriverException as e: + except URLError as e: Logger.error(UI_ERROR_MSG, e) except Exception as e: Logger.error("Encountered an unexpected error, please report the issue to the author", e) diff --git a/bggcli/ui/__init__.py b/bggcli/ui/__init__.py deleted file mode 100644 index 027e8ec..0000000 --- a/bggcli/ui/__init__.py +++ /dev/null @@ -1,51 +0,0 @@ -import os - -from selenium.webdriver.common.by import By - -from selenium.webdriver.support import expected_conditions as EC -from selenium.webdriver.support.select import Select -from selenium.webdriver.support.wait import WebDriverWait - - -class BasePage: - def __init__(self, driver): - self.driver = driver - # To wait for dynamic content (e.g. a popup), Sauce labs being much slower - if os.environ.get('CI') == 'true': - timeout = 60 - else: - timeout = 5 - self.wait = WebDriverWait(driver, timeout) - - @staticmethod - def update_text(el, value): - el.clear() - el.send_keys(value) - - @staticmethod - def update_checkbox(root_el, xpath, value): - elem = root_el.find_element_by_xpath(xpath) - if (elem.is_selected() and (value == '0')) or (not elem.is_selected() and (value == '1')): - elem.click() - - @staticmethod - def update_select(el, value, by_text=False): - select = Select(el) - if value == '': - select.select_by_index(0) - elif by_text: - select.select_by_visible_text(value) - else: - select.select_by_value(value) - - def update_textarea(self, root_el, fieldname, value): - root_el.find_element_by_xpath(".//td[@class='collection_%smod editfield']" % fieldname) \ - .click() - form = self.wait.until(EC.element_to_be_clickable( - (By.XPATH, ".//form[contains(@id, '%s')]" % fieldname))) - self.update_text(form.find_element_by_xpath(".//textarea"), value) - form.find_element_by_xpath(".//input[contains(@onclick, 'CE_SaveData')]").click() - - def wait_and_accept_alert(self): - self.wait.until(EC.alert_is_present()) - self.driver.switch_to.alert.accept() diff --git a/bggcli/ui/collectionpage.py b/bggcli/ui/collectionpage.py deleted file mode 100644 index 634f880..0000000 --- a/bggcli/ui/collectionpage.py +++ /dev/null @@ -1,26 +0,0 @@ -""" -bgg.collectionpage -~~~~~~~~~~~~ - -Selenium Page Object to bind the collection page - -""" - -from selenium.common.exceptions import NoSuchElementException - -from bggcli import BGG_BASE_URL -from bggcli.ui import BasePage - - -class CollectionPage(BasePage): - def is_empty(self, login): - """ - Returns True if the collection is empty, False otherwise - - :param login: User login - """ - self.driver.get("%s/collection/user/%s" % (BGG_BASE_URL, login)) - try: - self.driver.find_element_by_xpath("//td[contains(@class, 'collection_objectname')]") - except NoSuchElementException: - return True diff --git a/bggcli/ui/gamepage.py b/bggcli/ui/gamepage.py deleted file mode 100644 index 227ff78..0000000 --- a/bggcli/ui/gamepage.py +++ /dev/null @@ -1,282 +0,0 @@ -""" -bgg.gamepage -~~~~~~~~~~~~ - -Selenium Page Object to bind the game details page - -""" - -from selenium.common.exceptions import NoSuchElementException, WebDriverException -from selenium.webdriver.common.by import By -from selenium.webdriver.support import expected_conditions as EC - -from bggcli import BGG_BASE_URL, BGG_SUPPORTED_FIELDS -from bggcli.ui import BasePage -from bggcli.util.logger import Logger - - -def in_private_info_popup(func): - """ - Ensure the Private Info popup is displayed when updating its attributes - """ - - def _wrapper(self, *args, **kwargs): - try: - self.itemEl \ - .find_element_by_xpath(".//td[@class='collection_ownershipmod editfield']") \ - .click() - except NoSuchElementException: - pass - else: - self.privateInfoPopupEl = self.wait.until(EC.element_to_be_clickable( - (By.XPATH, "//div[@class='select-free'][contains(@id, 'editownership')]"))) - - return func(self, *args, **kwargs) - - return _wrapper - - -def in_version_popup(func): - """ - Ensure the Version popup is displayed when updating its attributes - """ - - def _wrapper(self, *args, **kwargs): - try: - self.itemEl \ - .find_element_by_xpath(".//td[@class='collection_versionmod editfield']") \ - .click() - except NoSuchElementException: - pass - else: - self.versionPopupEl = self.wait.until(EC.element_to_be_clickable( - (By.XPATH, "//div[@class='select-free'][contains(@id, 'editversion')]"))) - self.versionPopupEl \ - .find_element_by_xpath("//a[contains(@onclick, 'oldversion_version')]").click() - self.wait.until(EC.element_to_be_clickable( - (By.XPATH, "//div[contains(@id, 'oldversion_version')]"))) - - return func(self, *args, **kwargs) - - return _wrapper - - -class GamePage(BasePage): - # CSV_SUPPORTED_COLUMNS = [ - # 'objectid', 'rating', 'weight', 'own', 'fortrade', 'want', 'wanttobuy', 'wanttoplay', - # 'prevowned', 'preordered', 'wishlist', 'wishlistpriority', 'wishlistcomment', 'comment', - # 'conditiontext', 'haspartslist', 'wantpartslist', 'publisherid', 'imageid', - # 'year', 'language', 'other', 'pricepaid', 'pp_currency', 'currvalue', 'cv_currency', - # 'acquisitiondate', 'acquiredfrom', 'quantity', 'privatecomment', '_versionid' - # ] - - def __init__(self, driver): - BasePage.__init__(self, driver) - - self.itemEl = None - self.privateInfoPopupEl = None - self.versionPopupEl = None - - def goto(self, game_attrs): - """ - Set Web Driver on the game details page - - :param game_attrs: Game attributes as a dictionary - """ - self.driver.get("%s/boardgame/%s" % (BGG_BASE_URL, game_attrs['objectid'])) - - def update(self, game_attrs): - """ - Update game details - - :param game_attrs: Game attributes as a dictionary - """ - self.goto(game_attrs) - - try: - self.itemEl = self.driver.find_element_by_xpath( - "//table[@class='collectionmodule_table']") - Logger.info(" (already in collection)", append=True, break_line=False) - except NoSuchElementException: - self.driver.find_element_by_xpath( - "(//a[contains(@onclick, 'CE_ModuleAddItem')])[last()]").click() - self.itemEl = self.wait.until(EC.element_to_be_clickable( - (By.XPATH, "//table[@class='collectionmodule_table']"))) - - # Fill all provided values using dynamic invocations 'fill_[fieldname]' - for key in game_attrs: - if key in BGG_SUPPORTED_FIELDS: - value = game_attrs[key] - if value is not None: - getattr(self, "fill_%s" % key)(value) - - # Save "Private Info" popup if opened - try: - self.privateInfoPopupEl.find_element_by_xpath(".//input[@type='submit']").click() - except WebDriverException: - pass - else: - self.wait.until(EC.element_to_be_clickable( - (By.XPATH, ".//td[@class='collection_ownershipmod editfield']"))) - - # Save "Version" popup if opened - try: - self.versionPopupEl.find_element_by_xpath(".//input[@type='submit']").click() - except WebDriverException: - pass - else: - self.wait.until(EC.element_to_be_clickable( - (By.XPATH, ".//td[@class='collection_versionmod editfield']"))) - - def delete(self, game_attrs): - """ - Delete a game in the collection - - :param game_attrs: Game attributes as a dictionary. Only the id will be used - """ - self.goto(game_attrs) - try: - del_button = self.driver.find_element_by_xpath("//a[contains(@onclick, " - "'CE_ModuleDeleteItem')]") - except NoSuchElementException: - Logger.info(" (not in collection)", append=True, break_line=False) - return - - del_button.click() - - # Confirm alert message - self.wait_and_accept_alert() - - ############################################################################################### - # All following functions are invoked dynamically, for each attribute that can be updated # - # Functions are named 'fill_{attribute-name}' # - ############################################################################################### - - def fill_objectid(self, value): - pass - - def fill_objectname(self, value): - pass - - def fill_rating(self, value): - td = self.driver.find_element_by_xpath("//td[contains(@id, 'CEcell_rating')]") - td.click() - - self.update_text(self.wait.until( - EC.element_to_be_clickable((By.XPATH, "//input[@style='editrating']"))), value) - td.find_element_by_xpath(".//input[@type='submit']").click() - - def fill_weight(self, value): - self.update_select(self.itemEl.find_element_by_xpath(".//select[@name='weight']"), value) - - def fill_own(self, value): - self.update_checkbox(self.itemEl, ".//ul[@class='collectionstatus']//input[@name='own']", - value) - - def fill_fortrade(self, value): - self.update_checkbox(self.itemEl, - ".//ul[@class='collectionstatus']//input[@name='fortrade']", value) - - def fill_want(self, value): - self.update_checkbox(self.itemEl, ".//ul[@class='collectionstatus']//input[@name='want']", - value) - - def fill_wanttobuy(self, value): - self.update_checkbox(self.itemEl, - ".//ul[@class='collectionstatus']//input[@name='wanttobuy']", value) - - def fill_wanttoplay(self, value): - self.update_checkbox(self.itemEl, - ".//ul[@class='collectionstatus']//input[@name='wanttoplay']", value) - - def fill_prevowned(self, value): - self.update_checkbox(self.itemEl, - ".//ul[@class='collectionstatus']//input[@name='prevowned']", value) - - def fill_preordered(self, value): - self.update_checkbox(self.itemEl, - ".//ul[@class='collectionstatus']//input[@name='preordered']", value) - - def fill_wishlist(self, value): - self.update_checkbox(self.itemEl, ".//input[@name='wishlist']", value) - - def fill_wishlistpriority(self, value): - self.update_select( - self.itemEl.find_element_by_xpath(".//select[@name='wishlistpriority']"), value) - - def fill_wishlistcomment(self, value): - self.update_textarea(self.itemEl, 'wishlistcomment', value) - - def fill_comment(self, value): - self.update_textarea(self.itemEl, 'comment', value) - - def fill_conditiontext(self, value): - self.update_textarea(self.itemEl, 'conditiontext', value) - - def fill_haspartslist(self, value): - self.update_textarea(self.itemEl, 'haspartslist', value) - - def fill_wantpartslist(self, value): - self.update_textarea(self.itemEl, 'wantpartslist', value) - - @in_version_popup - def fill__versionid(self, value): - if value: - radio_version = self.versionPopupEl.find_element_by_xpath( - "(.//ul)[1]//input[@value='%s']" % value) - radio_version.click() - - @in_version_popup - def fill_publisherid(self, value): - self.update_text(self.versionPopupEl.find_element_by_name('publisherid'), value) - - @in_version_popup - def fill_imageid(self, value): - self.update_text(self.versionPopupEl.find_element_by_name('imageid'), value) - - @in_version_popup - def fill_year(self, value): - self.update_text(self.versionPopupEl.find_element_by_name('year'), value) - - @in_version_popup - def fill_language(self, value): - self.update_select(self.versionPopupEl.find_element_by_name('languageid'), value, - by_text=True) - - @in_version_popup - def fill_other(self, value): - self.update_text(self.versionPopupEl.find_element_by_name('other'), value) - - @in_private_info_popup - def fill_pricepaid(self, value): - self.update_text(self.privateInfoPopupEl.find_element_by_name('pricepaid'), value) - - @in_private_info_popup - def fill_pp_currency(self, value): - self.update_select(self.privateInfoPopupEl.find_element_by_name('pp_currency'), value) - - @in_private_info_popup - def fill_currvalue(self, value): - self.update_text(self.privateInfoPopupEl.find_element_by_name('currvalue'), value) - - @in_private_info_popup - def fill_cv_currency(self, value): - self.update_select(self.privateInfoPopupEl.find_element_by_name('cv_currency'), value) - - @in_private_info_popup - def fill_acquisitiondate(self, value): - self.update_text( - self.privateInfoPopupEl.find_element_by_xpath( - "//input[contains(@id, 'acquisitiondateinput')]"), value) - - @in_private_info_popup - def fill_acquiredfrom(self, value): - self.update_text(self.privateInfoPopupEl.find_element_by_name('acquiredfrom'), value) - - @in_private_info_popup - def fill_quantity(self, value): - self.update_text(self.privateInfoPopupEl.find_element_by_name('quantity'), value) - - @in_private_info_popup - def fill_privatecomment(self, value): - self.update_text(self.privateInfoPopupEl.find_element_by_name('privatecomment'), value) diff --git a/bggcli/ui/loginpage.py b/bggcli/ui/loginpage.py deleted file mode 100644 index 738c679..0000000 --- a/bggcli/ui/loginpage.py +++ /dev/null @@ -1,53 +0,0 @@ -""" -bgg.loginpage -~~~~~~~~~~~~ - -Selenium Page Object to bind the login page and perform authentication - -""" - -from selenium.common.exceptions import NoSuchElementException - -from bggcli import BGG_BASE_URL -from bggcli.ui import BasePage -from bggcli.util.logger import Logger - - -class LoginPage(BasePage): - def authenticate(self, login, password): - """ - Performs authentication - - :param login: BGG login - :param password: BGG password - """ - Logger.info("Authenticating...", break_line=False) - - self.driver.get("%s/login" % BGG_BASE_URL) - - # When user is already authenticated, just skip this task - # TODO Handle case where another user is logged in - if self.is_authenticated(login): - Logger.info(" (already logged) [done]", append=True) - return True - - self.update_text(self.driver.find_element_by_id("login_username"), login) - self.update_text(self.driver.find_element_by_id("login_password"), password) - self.driver.find_element_by_xpath("//div[@class='menu_login']//input[@type='submit']")\ - .click() - - if self.is_authenticated(login): - Logger.info(" [done]", append=True) - return True - - Logger.info(" [error]", append=True) - Logger.error("Authentication failed, check your credentials!") - return False - - def is_authenticated(self, login): - try: - self.driver.find_element_by_xpath("//div[@class='menu_login']//a[@href='/user/%s']" - % login) - return True - except NoSuchElementException: - return False diff --git a/bggcli/util/csvreader.py b/bggcli/util/csvreader.py index 99a534f..6108826 100644 --- a/bggcli/util/csvreader.py +++ b/bggcli/util/csvreader.py @@ -8,7 +8,7 @@ import csv -from selenium.common.exceptions import WebDriverException +from urllib2 import URLError from bggcli import UI_ERROR_MSG, BGG_SUPPORTED_FIELDS from bggcli.util.logger import Logger @@ -40,8 +40,9 @@ def iterate(self, callback): index = 1 for row in self.reader: objectid = row.get('objectid') - if objectid is None or not objectid.isdigit(): - Logger.error("No valid 'objectid' at line %s!" % index, None, sysexit=True) + collid = row.get('collid') + if collid is None or not (collid.isdigit() or (collid == "" and objectid.isdigit())): + Logger.error("No valid 'collid'/'objectid' at line %s!" % index, None, sysexit=True) return # Encode in UTF-8 @@ -52,13 +53,13 @@ def iterate(self, callback): objectname = row['objectname'] if objectname is None or objectname == "": - objectname = "(name not available for objectid=%s)" % objectid + objectname = "(name not available for objectid=%s, collid=%s)" % (objectid, collid) Logger.info("[%s/%s] %s... " % (index, self.rowCount, objectname), break_line=False) try: callback(row) - except WebDriverException as e: + except URLError as e: Logger.error(UI_ERROR_MSG, e, sysexit=True) return except Exception as e: diff --git a/bggcli/util/webdriver.py b/bggcli/util/webdriver.py deleted file mode 100644 index 820ebe9..0000000 --- a/bggcli/util/webdriver.py +++ /dev/null @@ -1,80 +0,0 @@ -""" -bgg.webdriver -~~~~~~~~~~~~ - -Utility in charge of wrapping Selenium web driver - -""" -import os - -from selenium import webdriver -from selenium.common.exceptions import WebDriverException -from selenium.webdriver import DesiredCapabilities -from bggcli.util.logger import Logger - - -class WebDriver: - # noinspection PyUnusedLocal,PyDefaultArgument - def __init__(self, name, args=[], options={}): - self.name = name - self.browser_keep = (options.get('browser-keep') == "true") - - # When used with Travis+SauceLabs or directly with SauceLabs - if os.environ.get('CI') == 'true': - self.driver = self.create_ci_driver() - else: - self.driver = self.create_local_firefox_driver(options.get('browser-profile-dir')) - - def __enter__(self): - return self - - # noinspection PyUnusedLocal,PyShadowingBuiltins - def __exit__(self, type, value, traceback): - if not self.browser_keep: - try: - self.driver.quit() - Logger.verbose("Close webdriver '%s'" % self.name) - except WebDriverException as e: - Logger.info("Encountered error when closing webdriver '%s' (will be skipped): %s" - % (self.name, repr(e))) - return type is None - - # noinspection PyMethodMayBeStatic - def create_local_firefox_driver(self, profile_path): - if profile_path is None: - profile = webdriver.FirefoxProfile() - else: - profile = webdriver.FirefoxProfile(profile_path) - - return webdriver.Firefox(firefox_profile=profile) - - # noinspection PyMethodMayBeStatic - def create_ci_driver(self): - # See http://docs.travis-ci.com/user/gui-and-headless-browsers/ - capabilities = DesiredCapabilities.FIREFOX.copy() - if os.environ.get('TRAVIS') == 'true': - Logger.info("Configure Travis for Sauce Labs") - capabilities.update( - { - 'tunnel-identifier': os.environ["TRAVIS_JOB_NUMBER"], - 'build': os.environ["TRAVIS_JOB_NUMBER"], - 'tags': [os.environ["TRAVIS_PYTHON_VERSION"], "CI"] - } - ) - sauce_url = "http://%s:%s@localhost:4445/wd/hub" %\ - ((os.environ["SAUCE_USERNAME"]), (os.environ["SAUCE_ACCESS_KEY"])) - else: - Logger.info("Configure direct usage of Sauce Labs") - sauce_url = "http://%s:%s@ondemand.saucelabs.com:80/wd/hub" %\ - ((os.environ["SAUCE_USERNAME"]), (os.environ["SAUCE_ACCESS_KEY"])) - - capabilities.update( - { - 'name': 'bggcli-%s' % self.name - } - ) - - result = webdriver.Remote(desired_capabilities=capabilities, command_executor=sauce_url) - result.implicitly_wait(20) - - return result diff --git a/bggcli/util/xmltocsv.py b/bggcli/util/xmltocsv.py index 42e3877..4d36d3d 100644 --- a/bggcli/util/xmltocsv.py +++ b/bggcli/util/xmltocsv.py @@ -66,6 +66,7 @@ def convert_item(el): 'objectname': XmlToCsv._to_str(el.find('name').text), 'objectid': el.attrib.get('objectid'), 'rating': XmlToCsv._zero_if_na(XmlToCsv._wrap(el.find('stats/rating')).get('value')), + 'numplays': XmlToCsv._to_int(el.find('numplays').text), 'own': XmlToCsv._to_int(status_atts.get('own')), 'fortrade': XmlToCsv._to_int(status_atts.get('fortrade')), 'want': XmlToCsv._to_int(status_atts.get('want')), @@ -80,10 +81,22 @@ def convert_item(el): 'conditiontext': XmlToCsv._to_str(XmlToCsv._wrap(el.find('conditiontext')).text), 'haspartslist': XmlToCsv._to_str(XmlToCsv._wrap(el.find('haspartslist')).text), 'wantpartslist': XmlToCsv._to_str(XmlToCsv._wrap(el.find('wantpartslist')).text), + 'collid': el.attrib.get('collid'), + 'baverage': XmlToCsv._zero_if_na(XmlToCsv._wrap(el.find('stats/rating/bayesaverage')).get('value')), + 'average': XmlToCsv._zero_if_na(XmlToCsv._wrap(el.find('stats/rating/average')).get('value')), + 'numowned': XmlToCsv._to_int(XmlToCsv._wrap(el.find('stats')).get('numowned')), + 'objecttype': el.attrib.get('objecttype'), + 'minplayers': XmlToCsv._to_int(XmlToCsv._wrap(el.find('stats')).get('minplayers')), + 'maxplayers': XmlToCsv._to_int(XmlToCsv._wrap(el.find('stats')).get('maxplayers')), + 'playingtime': XmlToCsv._to_int(XmlToCsv._wrap(el.find('stats')).get('playingtime')), + 'maxplaytime': XmlToCsv._to_int(XmlToCsv._wrap(el.find('stats')).get('maxplaytime')), + 'minplaytime': XmlToCsv._to_int(XmlToCsv._wrap(el.find('stats')).get('minplaytime')), + 'yearpublished': XmlToCsv._to_str(el.find('yearpublished').text), 'publisherid': XmlToCsv._to_str(publisher_atts.get('publisherid')), 'year': XmlToCsv._to_str(XmlToCsv._wrap(version_el.find('year')).text), 'imageid': XmlToCsv._to_str(XmlToCsv._wrap(version_el.find('imageid')).get('value')), 'other': XmlToCsv._to_str(XmlToCsv._wrap(version_el.find('other')).text), + 'barcode': XmlToCsv._to_str(XmlToCsv._wrap(version_el.find('barcode')).text), 'pricepaid': XmlToCsv._to_str(private_info_el.get('pricepaid')), 'pp_currency': XmlToCsv._to_str(private_info_el.get('pp_currency')), 'currvalue': XmlToCsv._to_str(private_info_el.get('currvalue')), diff --git a/setup.py b/setup.py index 90bacea..bb6c869 100644 --- a/setup.py +++ b/setup.py @@ -40,7 +40,7 @@ def run_tests(self): bggcli=bggcli.main:main ''', install_requires=[ - 'selenium', 'docopt' + 'docopt' ], keywords='bgg boardgamegeek', license='MIT', diff --git a/tests/commons.py b/tests/commons.py index 9075906..8dff1cd 100644 --- a/tests/commons.py +++ b/tests/commons.py @@ -4,7 +4,7 @@ PASSWORD = os.environ.get('BGGCLI_TEST_PASSWORD') COLLECTION_CSV_PATH = os.path.join(os.path.dirname(__file__), 'resources/collection.csv') COLLECTION_XML_PATH = os.path.join(os.path.dirname(__file__), 'resources/collection.xml') -CSV_COLUMN_TO_IGNORE = ['language'] +CSV_COLUMN_TO_IGNORE = ['collid', 'baverage', 'average', 'numowned', 'language'] def debug_test(msg=""): diff --git a/tests/resources/collection.csv b/tests/resources/collection.csv index f64c648..150b201 100644 --- a/tests/resources/collection.csv +++ b/tests/resources/collection.csv @@ -1,3 +1,3 @@ -objectname,objectid,rating,own,fortrade,want,wanttobuy,wanttoplay,prevowned,preordered,wishlist,wishlistpriority,wishlistcomment,comment,conditiontext,haspartslist,wantpartslist,publisherid,imageid,year,language,other,pricepaid,pp_currency,currvalue,cv_currency,acquisitiondate,acquiredfrom,quantity,privatecomment,_versionid -"7 Wonders","68448","9","1","0","0","0","0","0","0","0","","Some wish list comment","Some public comment","Some trade condition","Some hasparts comment","Some wantparts comment","","","","","","35","USD","36","USD","2014-01-17","Amazon","1","Some private comment","107117" -"Age of Steam","4098","0","0","0","0","0","0","0","0","1","3","Some wish list comment","","","","","888","999","2000",French,"other-version","","","","","","","","","" +objectname,objectid,rating,numplays,own,fortrade,want,wanttobuy,wanttoplay,prevowned,preordered,wishlist,wishlistpriority,wishlistcomment,comment,conditiontext,haspartslist,wantpartslist,collid,baverage,average,numowned,objecttype,minplayers,maxplayers,playingtime,maxplaytime,minplaytime,yearpublished,publisherid,imageid,year,language,other,barcode,pricepaid,pp_currency,currvalue,cv_currency,acquisitiondate,acquiredfrom,quantity,privatecomment,_versionid +"7 Wonders","68448","9","0","1","0","0","0","0","0","0","0","","Some wish list comment","Some public comment","Some trade condition","Some hasparts comment","Some wantparts comment","","7.79762","7.87677","42851","thing","2","7","30","30","30","2011","","","","","","","35","USD","36","USD","2014-01-17","Amazon","1","Some private comment","107117" +"Age of Steam","4098","0","0","0","0","0","0","0","0","0","1","3","Some wish list comment","","","","","","7.51","7.73199","5587","thing","1","6","120","120","120","2002","888","999","2000",French,"other-version","","","","","","","","","","" diff --git a/tests/test_collection.py b/tests/test_collection.py index 0e2d2b5..4bf5e7a 100644 --- a/tests/test_collection.py +++ b/tests/test_collection.py @@ -2,8 +2,6 @@ from bggcli.commands import collection_export from bggcli.main import _main -from bggcli.ui.collectionpage import CollectionPage -from bggcli.util.webdriver import WebDriver from commons import * @@ -39,29 +37,48 @@ def test_collection(tmpdir): # debug_test() - debug_test("1. Ensure the collection is empty") - with WebDriver('test_collection-empty') as web_driver: - if not CollectionPage(web_driver.driver).is_empty(LOGIN): - try: - _main(['-v', '--login', LOGIN, '--password', PASSWORD, - 'collection-delete', '--force', COLLECTION_CSV_PATH]) - except BaseException as e: - assert False, "Delete command should not fail: %s" % e - - assert CollectionPage(web_driver.driver).is_empty(LOGIN), 'Collection should be empty!' - debug_test("-> [ok (collection was not empty)]") - else: - debug_test("-> [ok (collection was empty)]") + debug_test("1. Fetch current collection as CSV") + delete_list_file = tmpdir.join('delete-collection.csv').strpath + assert not os.path.exists(delete_list_file) + _main(['-v', '--login', LOGIN, '--password', PASSWORD, + 'collection-export', delete_list_file]) + assert os.path.isfile(delete_list_file) + debug_test("-> [ok]") + + # + debug_test() + debug_test("2. Delete everything from collection") + try: + _main(['-v', '--login', LOGIN, '--password', PASSWORD, + 'collection-delete', '--force', delete_list_file]) + except BaseException as e: + assert False, "Delete command should not fail: %s" % e + # + debug_test() + debug_test("3. Test that collection is empty") + delete_check_file = tmpdir.join('delete-check-collection.csv').strpath + assert not os.path.exists(delete_check_file) + _main(['-v', '--login', LOGIN, '--password', PASSWORD, + 'collection-export', delete_check_file]) + reader_check_empty = csv.DictReader(open(delete_check_file, 'rU')) + try: + reader_check_empty.next() + assert False, 'Collection should be empty!' + except StopIteration: + pass + debug_test("-> [ok]") + + # debug_test() - debug_test("2. Import collection") + debug_test("4. Import collection") _main(['-v', '--login', LOGIN, '--password', PASSWORD, 'collection-import', COLLECTION_CSV_PATH]) debug_test("-> [ok]") # debug_test() - debug_test("3. Export collection as CSV") + debug_test("5. Export collection as CSV") actual_file = tmpdir.join('actual-collection.csv').strpath assert not os.path.exists(actual_file) _main(['-v', '--login', LOGIN, '--password', PASSWORD, @@ -71,7 +88,7 @@ def test_collection(tmpdir): # debug_test() - debug_test("4. Compare CSV files") + debug_test("6. Compare CSV files") compare_csv_files(actual_file, COLLECTION_CSV_PATH) debug_test("Comparison OK!") From 83db7c0b3998ea6be533e75c3f5536f69e0ab6a8 Mon Sep 17 00:00:00 2001 From: Stefan Siegel Date: Fri, 20 Nov 2015 19:24:23 +0100 Subject: [PATCH 2/2] Update README to use --force-new for collection migration There might be other viable alternatives: - Create new copies by default, and introduce some --update switch to update the existing collection. - Add a completely new command for updating the collection, like e.g. collection-update. - Detect if the "collid" to be imported is already in the collection. Then update if it is, or add a copy if it is not. This solution probably exhibits the least surprising behavior for the user, but it requires an additional request per imported record or a complete export beforehand. --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 9afe78e..d3544f2 100644 --- a/README.rst +++ b/README.rst @@ -49,7 +49,7 @@ Type ``bggcli`` to get the full help about available commands. Here is an example to export a collection from an account *account1* and import it to another account *account2*:: $ bggcli -l mylogin1 -p mypassword1 collection-export mycollection.csv - $ bggcli -l mylogin2 -p mypassword2 collection-import mycollection.csv + $ bggcli -l mylogin2 -p mypassword2 collection-import --force-new mycollection.csv Update a collection -------------------