db: cache SCM instance short-term (tied to SQLAlchemy session lifetime)
Repeatedly checking whether SCM instances are invalidated is slow, and we don't actually _want_ SCM instances to invalidate half-way through a request either.
Therefore cache them in on the db.Repository object, the lifetime of which is directly tied to the lifetime of the SQLAlchemy session, the lifetime of which is tied directly to the individual HTTP request. This way, we only check for invalidation the first time the SCM instance is accessed in a request.
This will improve performance in cases where we have (by definition) badly written code that retrieves repo objects several times.
pullrequests: optimize iteration over reviewers - avoid fetching users one by one
.reviewers was mainly used for iteration and then dereferencing .user one by one. That gave lots of queries and round trips to the database and was slow.
Instead, do something else. Either query directly or use a new method for getting the list of reviewer users.
Reviewers will explicitly be shown in the order they have been added (assuming database id's are monotonic).
This method is not referenced anywhere in the Kallithea or Paste code. Additionally, calling it with any true-ish `log` argument would trigger a TypeError exception.
python-pam 1.8.* provides a new API replacing the previously available authenticate() function. The latest unreleased version brings it back, but until it's available, use a custom shim instead.
tests: clarify user IP range test dependency on beaker caching of user objects
Requests with invalid request address would pass after configuring user IP ranges because the IP range would not be validated as long as the user object was found in the beaker cache.
Instead, wait until the beaker cache has expired and verify the user cannot log in without a valid IP. Then provide a valid IP for later requests until the IP range is removed again.
Based on original patch and research by Dominik Ruf.
graph: detect git branches and colourise them properly without rainbow effect (Issue #188)
This is based on research and patches by Andrew Shadura.
When using Git, only commits pointed to by branch references have their branches set. In general, there's no way in Git to find out which branch a commit belongs to, so we can try to deduce that from the merge topology.
In other words, if we know the branch name, we know it's a different colour than any different branch. If we don't (it is None), we guesstimate the first parent is probably on the same branch. The relevant part of the code before 2da0dc09 used a similar, yet simpler, algorithm.
graph: avoid collision between "heir has been found" and "child has no name" both using None as branch name
Instead of using None for both, introduce a new flag for searching for heir.
This will do that Git changesets without branch name will inherit colors from each other instead of getting a new one every time. (Inheriting from from changesets with branch name is still phony.)
summary: add tooltip and link to status change icons (same as on changelog page)
This change makes the changeset list on the summary page more similar to the one on the changeset list. It adds the same tooltip ("Changeset status") to the changeset status change icons that is already found on the changelog page. It also adds a link to the first comment (as in the changelog page).
pullrequest: add URL changesets in txt notification email to reviewers
Similar to adding the URL for each changeset in the html notification email (commit xxxxxxxxxxxx) do the same for txt emails. The e-mail client presumably makes these URLs clickable.
vcs: better handling of invalid email addresses: don't consider them email addresses
13da89053853 was in principle right in always returning email adresses as string ... but unfortunately the function also returned invalid email addresses that didn't fit into strings.
To fix this, the function is refactored to always use regexp matching of valid email addresses ... and to be simpler. The behaviour should be the same as before for all valid email addresses.
pytest migration: files: convert to TestControllerPytest and parametrize
Migrate the 'files' tests to pytest and its parametrize decorator. The syntax of this decorator is slightly different: a first argument is a comma-separated string with the method argument names for which the values are provided as second argument.
pytest migration: make pytest's parametrize functionality available
To provide parameterized tests, a custom implementation is currently provided at kallithea/tests/parameterized.py because nose does not provide that out-of-the-box. pytest, on the other hand, does have a built-in 'parametrize' (note: different spelling) functionality. Therefore, once all tests have been converted to pytest, we can get rid of the custom 'parameterized' implementation. Also, the existing 'parameterized' implementation does not seem to work under pytest-style tests.
This commit makes pytest's 'parametrize' decorator available alongside the custom 'parameterized' decorator. The names are confusing but it is the intention to kill the original 'parameterized' soon.
pytest migration: convert simple functional tests to TestControllerPytest
Replace usage of TestController with TestControllerPytest for those files in tests/functional where there is no setUp/tearDown method (the pytest equivalent to be investigated) and that do not use test parametrization.
pytest migration: search: use tmpdir fixture instead of 'current dir'
The test_empty_search method needs to refer to an existing directory that is guaranteed to not contain a search index. The current implementation chose the 'current' directory '.' but it is more of a hack than careful planning. A temporary empty directory would be cleaner but was more involved to create.
With the introduction of pytest-style test classes, this can very easily be improved. Creating a temporary directory with pytest is as simple as accepting the magic 'tmpdir' argument to the test method. This 'tmpdir' is a fixture provided by pytest. The variable is initialized with a py.path.local object referring a temporary directory. For details, see: http://pytest.org/latest/tmpdir.html
In order to allow tests to benefit from pytest specific functionality, like fixtures, they can no longer derive from unittest.TestCase. What's more, while they can derive from any user-defined class, none of the classes involved (the test class itself nor any of the base classes) can have an __init__ method.
Converting all tests from unittest-style to pytest-style in one commit is not realistic. Hence, a more gradual approach is needed.
Most existing test classes derive from TestController, which in turn derives from BaseTestCase, which derives from unittest.TestCase. Some test classes derive directly from BaseTestCase. Supporting both unittest-style and pytest-style from TestController directly is not possible: pytest-style _cannot_ and unittest-style _must_ derive from unittest.TestCase. Thus, in any case, an extra level in the class hierarchy is needed (TestController deriving from Foo and from unittest.TestCase; pytest-style test classes would then directly derive from Foo).
The requirement that pytest-style test classes cannot have an __init__ method anywhere in the class hierarchy imposes another restriction that makes it difficult to support both unittest-style and pytest-style test classes with one class. Any init code needs to be placed in another method than __init__ and be called explicitly when the test class is initialized. For unittest-style test classes this would naturally be done with a setupClass method, but several test classes already use that. Thus, there would need to be explicit 'super' calls from the test classes. This is technically possible but not very nice.
A more transparent approach (from the existing test classes point of view), implemented by this patch, works as follows: - the implementation of the existing TestController class is now put under a new class BaseTestController. To accomodate pytest, the __init__ method is renamed init. - contrary to the original TestController, BaseTestController does not derive from BaseTestCase (and neither from unittest.TestCase). Instead, the 'new' TestController derives both from BaseTestCase, which is untouched, and from BaseTestController. - TestController has an __init__ method that calls the base classes' __init__ methods and the renamed 'init' method of BaseTestController. - a new class TestControllerPytest is introduced that derives from BaseTestController but not from BaseTestCase. It uses a pytest fixture to automatically call the setup functionality previously provided by BaseTestCase and also calls 'init' on BaseTestController. This means a little code duplication but is hard to avoid.
The app setup fixture is scoped on the test method, which means that the app is recreated for every test (unlike for the unittest-style tests where the app is created per test class). This has the advantage of detecting current inter-test dependencies and thus improve the health of our test suite. This in turn is one step closer to allowing parallel test execution.
The unittest-style assert methods (assertEqual, assertIn, ...) do not exist for pytest-style tests. To avoid having to change all existing test cases upfront, provide transitional implementations of these methods. The conversion of the unittest asserts to the pytest/python asserts can happen gradually over time.
Test test_index_trending depended on previous actions performed by the test test_index_statistics. Such inter-test dependencies should not be present.
This would become a problem when converting the test class to pytest-style, because the app is recreated for each test, while the original TestController creates it once per class.
tests: move remove_all_notifications outside of BaseTestCase
In preparation of allowing real pytest-style test cases (instead of unittest-style ones), some reorganization is needed in the base test classes, for one because we want a transition period where pytest and unittest style test cases can live alongside each other, and secondly because the pytest style test classes cannot have an __init__ method.
The BaseTestCase class will not be reused for the pytest test cases, but the remove_all_notifications method will. To avoid having to duplicate it, and since it does not use any resources from the class (self), move the method out of the BaseTestCase class to top-level, and export it in kallithea.tests.
pytest migration: use pytest test skipping instead of nose's
While the nose test skipping still worked, we want to remove all remnants of nose, so switch to the pytest equivalent.
Note: due to a bug in pytest-sugar (reported at [1]) the skipped tests are not shown explicitly when using the 'skipif' approach, while they were with the nose approach. Since pytest-sugar is not enabled yet in setup.py, this is not deemed a problem.
tests: gists: comment out always-skipped test of put functionality
There is little sense in always skipping a test because the tested functionality is not implemented. Instead, comment out the test and leave it to the future implementer to re-enable it.
middleware: allow git and hg users to use email address instead of the username
This commit also replaces __get_user('default') with a call to a more widely used User.get_default_user() function, and removes no longer really used __get_user() methods from both SimpleHg and SimpleGit.
auth: authenticate using either username or email address
Use User.get_by_username_or_email() in get_user. In authenticate(), update username if get_user succeeds.
The point of this change is that the web login is a complex thing that includes, apart the authentication itself, form validation and a bunch of other things.
This change on its own makes it possible to authenticate a user using its email address, but that on its own isn't enough for web login or git/hg auth.
db: match case-insensitively using func.lower, not ilike
ilike() uses SQL ILIKE operator internally, which means it interprets '%' and '_' in the match pattern as wildcards. Instead of ilike(), it's better to turn both operands to the lower case and compare them.
This also unbreaks the test case introduced in 13d0fe6f751a.
This commit removes case-sensitive email matching. It also adds a couple of tests which fail, to demonstrate a defect in the current implementation (using ILIKE matching instead of case-insensitive equality comparison).
pullrequests: show tags in lists of included and available changesets
Further improvement: Also show bookmarks and other names (preferably by using some helper function/template instead of duplicating code). It would perhaps also be better to avoid using floating.
The table is populated on database creation, and assumed to be populated so many places in the code, we're unlikely to even reach this point if it's empty. (E.g. web.push_ssl must be defined to push/pull/fetch both Mercurial and Git repositories.)
Only newly created objects (and objects explicitly expunged) need to be added to the SQLAlchemy session; any object returned from a database query is already in the session.
comments: change comment formatting to plain text instead of rst
There might be value in enabling rich markup (especially if it is markdown instead of rst) ... or it might be a waste of time. We might revisit that later.
But either way: Changing to plain text makes it more feasible to do markup of hashes and issues - for now that is more important than rtf.
Eventually this function should support and auto detect multiple formats and is thus not named for a specific format. But for now it is plain text only.
This kind of markup can quite easily and safely support additional magic markup. It is much harder to do that on top of a richer markup format; it must essentially be done in a single pass, with both all the various regexps and the rst formatting done in a single pass.