diff --git a/ckan/lib/repoze_patch.py b/ckan/lib/repoze_patch.py index 349be98db4f..0efde8f6484 100644 --- a/ckan/lib/repoze_patch.py +++ b/ckan/lib/repoze_patch.py @@ -1,17 +1,13 @@ from webob import Request, Response - -import openid -from openid.store import memstore, filestore, sqlstore from openid.consumer import consumer -from openid.oidutil import appendArgs -from openid.cryptutil import randomString -from openid.fetchers import setDefaultFetcher, Urllib2Fetcher -from openid.extensions import pape, sreg, ax +from openid.extensions import sreg, ax + +import lib.helpers as h # #1659 fix - logged_out_url prefixed with mount point def get_full_path(path, environ): if path.startswith('/'): - path = environ.get('SCRIPT_NAME', '') + path + path = h._add_i18n_to_url(path) return path def identify(self, environ): @@ -23,6 +19,7 @@ def identify(self, environ): """ request = Request(environ) + logger = environ['repoze.who.logger'] # #1659 fix - Use PATH_INFO rather than request.path as the former # strips off the mount point. path = environ['PATH_INFO'] @@ -36,7 +33,7 @@ def identify(self, environ): res.status = 302 res.location = get_full_path(self.logged_out_url, environ) - + environ['repoze.who.application'] = res return {} @@ -48,18 +45,19 @@ def identify(self, environ): # when the openid provider redirects the user back. if path == self.login_handler_path: - # in the case we are coming from the login form we should have + # in the case we are coming from the login form we should have # an openid in here the user entered open_id = request.params.get(self.openid_field, None) - if environ['repoze.who.logger'] is not None: - environ['repoze.who.logger'].debug('checking openid results for : %s ' %open_id) + if logger: + logger.debug('checking openid results for : %s ' %open_id) if open_id is not None: open_id = open_id.strip() # we don't do anything with the openid we found ourselves but we put it in here # to tell the challenge plugin to initiate the challenge - identity['repoze.whoplugins.openid.openid'] = environ['repoze.whoplugins.openid.openid'] = open_id + identity['repoze.whoplugins.openid.openid'] = open_id + environ['repoze.whoplugins.openid.openid'] = open_id # this part now is for the case when the openid provider redirects # the user back. We should find some openid specific fields in the request. @@ -87,7 +85,8 @@ def identify(self, environ): except KeyError: pass - environ['repoze.who.logger'].info('openid request successful for : %s ' %open_id) + if logger: + logger.info('openid request successful for : %s ' %open_id) display_identifier = info.identity_url @@ -102,7 +101,7 @@ def identify(self, environ): return identity # TODO: Do we have to check for more failures and such? - # + # elif mode=="cancel": # cancel is a negative assertion in the OpenID protocol, # which means the user did not authorize correctly. @@ -121,12 +120,12 @@ def redirect_to_logged_in(self, environ): res = Response() res.status = 302 res.location = url - environ['repoze.who.application'] = res + environ['repoze.who.application'] = res def challenge(self, environ, status, app_headers, forget_headers): """the challenge method is called when the ``IChallengeDecider`` - in ``classifiers.py`` returns ``True``. This is the case for either a - ``401`` response from the client or if the key + in ``classifiers.py`` returns ``True``. This is the case for either a + ``401`` response from the client or if the key ``repoze.whoplugins.openid.openidrepoze.whoplugins.openid.openid`` is present in the WSGI environment. The name of this key can be adjusted via the ``openid_field`` configuration @@ -140,11 +139,12 @@ def challenge(self, environ, status, app_headers, forget_headers): TODO: make the environment key to check also configurable in the challenge_decider. - For the OpenID flow check `the OpenID library documentation + For the OpenID flow check `the OpenID library documentation `_ """ request = Request(environ) + logger = environ['repoze.who.logger'] # check for the field present, if not redirect to login_form if not request.params.has_key(self.openid_field): @@ -154,10 +154,10 @@ def challenge(self, environ, status, app_headers, forget_headers): res.location = get_full_path(self.login_form_url, environ)+"?%s=%s" %(self.came_from_field, request.url) return res - # now we have an openid from the user in the request + # now we have an openid from the user in the request openid_url = request.params[self.openid_field] - if environ['repoze.who.logger'] is not None: - environ['repoze.who.logger'].debug('starting openid request for : %s ' %openid_url) + if logger: + logger.debug('starting openid request for : %s ' %openid_url) try: # we create a new Consumer and start the discovery process for the URL given @@ -180,12 +180,14 @@ def challenge(self, environ, status, app_headers, forget_headers): except consumer.DiscoveryFailure, exc: # eventually no openid server could be found environ[self.error_field] = 'Error in discovery: %s' %exc[0] - environ['repoze.who.logger'].info('Error in discovery: %s ' %exc[0]) + if logger: + logger.info('Error in discovery: %s ' %exc[0]) return self._redirect_to_loginform(environ) except KeyError, exc: # TODO: when does that happen, why does plone.openid use "pass" here? environ[self.error_field] = 'Error in discovery: %s' %exc[0] - environ['repoze.who.logger'].info('Error in discovery: %s ' %exc[0]) + if logger: + logger.info('Error in discovery: %s ' %exc[0]) return self._redirect_to_loginform(environ) return None @@ -193,7 +195,8 @@ def challenge(self, environ, status, app_headers, forget_headers): # should actually been handled by the DiscoveryFailure exception above if openid_request is None: environ[self.error_field] = 'No OpenID services found for %s' %openid_url - environ['repoze.who.logger'].info('No OpenID services found for: %s ' %openid_url) + if logger: + logger.info('No OpenID services found for: %s ' %openid_url) return self._redirect_to_loginform(environ) # we have to tell the openid provider where to send the user after login @@ -207,8 +210,8 @@ def challenge(self, environ, status, app_headers, forget_headers): # return_to is the actual URL to be used for returning to this app return_to = request.path_url # we return to this URL here trust_root = request.application_url - if environ['repoze.who.logger'] is not None: - environ['repoze.who.logger'].debug('setting return_to URL to : %s ' %return_to) + if logger: + logger.debug('setting return_to URL to : %s ' %return_to) # TODO: usually you should check openid_request.shouldSendRedirect() # but this might say you have to use a form redirect and I don't get why @@ -216,13 +219,13 @@ def challenge(self, environ, status, app_headers, forget_headers): # TODO: we might also want to give the application some way of adding # extensions to this message. - redirect_url = openid_request.redirectURL(trust_root, return_to) + redirect_url = openid_request.redirectURL(trust_root, return_to) # # , immediate=False) res = Response() res.status = 302 res.location = redirect_url - if environ['repoze.who.logger'] is not None: - environ['repoze.who.logger'].debug('redirecting to : %s ' %redirect_url) + if logger: + logger.debug('redirecting to : %s ' %redirect_url) # now it's redirecting and might come back via the identify() method # from the openid provider once the user logged in there. diff --git a/doc/test.rst b/doc/test.rst index 87ca6dfb03e..d88ed047d0c 100644 --- a/doc/test.rst +++ b/doc/test.rst @@ -2,26 +2,27 @@ Testing for Developers ====================== -If you are installing CKAN from source, or developing extensions, then you need to know how to run CKAN tests. - -This section describes testing topics for developers, including basic tests, migration testing and testing against PostgreSQL. +If you're a CKAN developer, if you're developing an extension for CKAN, or if +you're just installing CKAN from source, you should make sure that CKAN's tests +pass for your copy of CKAN. This section explains how to run CKAN's tests. .. _basic-tests: -Basic Tests ------------ - -After completing your source installation of CKAN, you should check that tests pass. You should also check this before checking in changes to CKAN code. +Installing Additional Dependencies +---------------------------------- -Make sure you've created a config file at ``pyenv/ckan/development.ini``. Then activate the Python environment:: +Some additional dependencies are needed to run the tests. Make sure you've +created a config file at ``pyenv/ckan/development.ini``, then activate your +virtual environment:: . pyenv/bin/activate -Install nose and other test-specific dependencies into your virtual environment:: +Install nose and other test-specific CKAN dependencies into your virtual +environment:: pip install --ignore-installed -r pyenv/src/ckan/pip-requirements-test.txt -At this point you will need to deactivate and then re-activate your +At this point you'll need to deactivate and then re-activate your virtual environment to ensure that all the scripts point to the correct locations: @@ -30,85 +31,99 @@ locations: deactivate . pyenv/bin/activate -Then run the quick development tests:: +Testing with SQLite +------------------- + +To run the CKAN tests using SQLite as the database library:: cd pyenv/src/ckan - nosetests ckan/tests --ckan + nosetests --ckan ckan You *must* run the tests from the CKAN directory as shown above, otherwise the -``--ckan`` plugin won't work correctly. - -.. warning :: +``--ckan`` plugin won't work correctly. - By default, the test run is 'quick and dirty' - only good enough as an initial check. +In deployment CKAN uses PostgreSQL, not SQLite. Running the tests with SQLite +is less thorough but much quicker than with PostgreSQL, good enough for an +initial check but you should run the tests with PostgreSQL before deploying +anything or releasing any code. +Testing Core Extensions +``````````````````````` -Testing against PostgreSQL --------------------------- - -The default way to run tests is defined in ``test.ini`` (which is the default config file for nose - change it with option ``--with-pylons``). This specifies using SQLite and sets ``faster_db_test_hacks``, which are compromises. - -:: +CKAN's core extensions (those extensions that are kept in the CKAN codebase +alongside CKAN itself) have their own tests. For example, to run the tests for +the stats extension do:: - cd pyenv/src/ckan - nosetests ckan/tests --ckan + nosetests --ckan ckanext/stats -Although SQLite is useful for testing a large proportion of CKAN, actually in deployment, CKAN must run with PostgreSQL. +To run the tests for all of the core extensions at once:: -Running the tests against PostgreSQL is slower but more thorough for two reasons: + nosetests --ckan ckanext - 1. You test subtleties of PostgreSQL - 2. CKAN's default search relies on PostgreSQL's custom full-text search, so these (100 or so) tests are skipped when running against SQLite. +Or to run the CKAN tests and the core extensions tests together:: -So when making changes to anything involved with search or closely related to the database, it is wise to test against PostgreSQL. + nosetests --ckan ckan ckanext -To test against PostgreSQL: +Testing with PostgreSQL +----------------------- - 1. Edit your local ``development.ini`` to specify a PostgreSQL database with the ``sqlalchemy.url`` parameter. - 2. Tell nose to use ``test-core.ini`` (which imports settings from ``development.ini``) +First, make sure you have specified a PostgreSQL database with the +``sqlalchemy.url`` parameter in your ``development.ini`` file. -:: +CKAN's default nose configuration file (``test.ini``) specifies SQLite as the +database library (it also sets ``faster_db_test_hacks``). To run the tests more +thoroughly with PostgreSQL, specify the ``test-core.ini`` nose configuration +file instead, for example:: - nosetests ckan/tests --ckan --with-pylons=test-core.ini - -The test suite takes a long time to run against standard PostgreSQL (approx. 15 minutes, or close to an hour on Ubuntu/10.04 Lucid). + nosetests --ckan --with-pylons=test-core.ini ckan + nosetests --ckan --with-pylons=test-core.ini ckanext/stats + nosetests --ckan --with-pylons=test-core.ini ckanext + nosetests --ckan --with-pylons=test-core.ini ckan ckanext -This can be improved to between 5 and 15 minutes by running PostgreSQL in memory and turning off durability, as described `in the PostgreSQL documentation `_. +The speed of the PostgreSQL tesrs can be improved by running PostgreSQL in +memory and turning off durability, as described +`in the PostgreSQL documentation `_. .. _migrationtesting: Migration Testing ----------------- -If your changes require a model change, you'll need to write a migration script. To ensure this is tested as well, you should instead run the tests this way:: +If you're a CKAN developer or extension developer and your new code requires a +change to CKAN's model, you'll need to write a migration script. To ensure that +the migration script itself gets tested, you should run the tests with +they ``--ckan-migration`` option, for example:: - nosetests ckan/tests --ckan --ckan-migration --with-pylons=test-core.ini - -By default, tests are run using the model defined in ``ckan/model``, but by using the ``--ckan-migration`` option the tests will run using a database that has been created using the migration scripts, which is the way the database is created and upgraded in production. These tests are the most thorough and will take around 20 minutes. + nosetests --ckan --ckan-migration --with-pylons=test-core.ini ckan + +By default tests are run using the model defined in ``ckan/model``. +With the ``--ckan-migration`` option the tests will run using a database that +has been created by running the migration scripts in ``ckan/migration``, which +is how the database is created and upgraded in production. .. caution :: - Ordinarily, you should set ``development.ini`` to specify a PostgreSQL database - so these also get used when running ``test-core.ini``, since ``test-core.ini`` - inherits from ``development.ini``. If you were to change the ``sqlalchemy.url`` - option in your ``development.ini`` file to use SQLite, the command above would - actually test SQLite rather than PostgreSQL, so always check the setting in - ``development.ini`` to ensure you are running the full tests. + Ordinarily, you should set ``development.ini`` to specify a PostgreSQL + database so these also get used when running ``test-core.ini``, since + ``test-core.ini`` inherits from ``development.ini``. If you were to change + the ``sqlalchemy.url`` option in your ``development.ini`` file to use + SQLite, the command above would actually test SQLite rather than + PostgreSQL, so always check the setting in ``development.ini`` to ensure + you are running the full tests. .. warning :: - A common error when wanting to run tests against a particular database is to change ``sqlalchemy.url`` in ``test.ini`` or ``test-core.ini``. The problem is that these are versioned files and people have checked in these by mistake, creating problems for other developers and the CKAN buildbot. This is easily avoided by only changing ``sqlalchemy.url`` in your local ``development.ini`` and testing ``--with-pylons=test-core.ini``. - -Testing Core Extensions ------------------------ - -Some extensions are in the CKAN core codebase and have their own suite of tests. For example:: - - nosetests --ckan ckanext/stats/tests + A common error when wanting to run tests against a particular database is to + change ``sqlalchemy.url`` in ``test.ini`` or ``test-core.ini``. The problem + is that these are versioned files and people have checked in these by + mistake, creating problems for other developers and the CKAN buildbot. This + is easily avoided by only changing ``sqlalchemy.url`` in your local + ``development.ini`` and testing ``--with-pylons=test-core.ini``. Common error messages --------------------- -Often errors are due to set-up errors. Always refer to the CKAN buildbot as the canonical build. +Often errors are due to set-up errors. Always refer to the CKAN buildbot as the +canonical build. -Consult :doc:`common-error-messages` for solutions to a range of setup problems. \ No newline at end of file +Consult :doc:`common-error-messages` for solutions to a range of setup problems.