Age | Commit message (Collapse) | Author |
|
|
|
|
|
|
|
Like the earlier added limit on number of tags on a given feed, this is
necessary to reduce DoS potential.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
I need to add documentation at some point too.
|
|
Replacing the old approach where for every entry the title & tags were
queried separately. If 50 entries are displayed, this is 100 extra
queries that are now one query. If the maximum 1000 are displayed, this
is 2000 queries to one.
Benchmarking this change with
ab -n 500 -c 100 http://localhost:8000/?page_num=1&per_page=1000
with 1000+ entries present, we get:
(PREVIOUS) Time taken for tests: 0.840 seconds
(NEW) Time taken for tests: 0.476 seconds
The crash occurred when get_feeds() was called with get_tags=True and
the query returned at least one feed with no tags. This was not handled
properly by later code. It was not noticed when testing the previous
commit since all feeds in the test instance had one or more tags.
|
|
|
|
This also avoids the need to pass `core` to the template, which I never
liked since it is basically giving up on encapsulation and passing the
whole world (as far as the application is concerned) to the template -
not only the values it needs.
But that could be avoided in other ways too without reducing the number
of queries from 1 + (number of feeds shown) - eg. 51 - to just 2, which
this does.
The next place something like this needs to be done is with the /
(index.tpl) view. Currently, that is also passed `core` and actually
does 2 * (number of entries shown) + 1 queries, which could be eg. 101,
since we fetch both the feed title and feed tags for every entry
separately.
With some improvements, it should be possible to do that too in 2
queries.
|
|
|
|
This is necessary to avoid a condition where the most recently added
feed is deleted, and a new one subsequently added, after the latest id
has been fetched from the database for updating but before the new
entries are inserted. In this case, without autoincrement, entries from
the deleted feed would be associated with the newly added feed.
This fix does not include migration from the old DB schema without
the change since this project is not stable yet, but I intend to start
including such migrations in perhaps the very next release that alters
the database schema.
|
|
Give the datetime attribute the UTC time ending in "Z" as prescribed.
Before, we were giving it the local time in the server's time zone,
which is wrong if the client is in a different time zone. This does not
change what is visually presented in the browser.
|
|
Also use input type "hidden" instead of display: none where appropriate
(no observable effects, just cleaner).
|
|
|
|
The approach we used is to catch an exception since EAFP. The
alternative would be to check if the feed exists just before trying to
insert, which would not be a race condition due to the lock.
|
|
That was the default value of that parameter anyway.
|
|
|
|
Seems necessary to have a limit here to avoid DoS, though of course there are
several avenues for it still, which should also be addressed. 100 seems more
than would be necessary for any non-pathological usage; the system is built
under the assumption that tags can be loaded all at once, don't need to be
paginated, etc. (unlike feeds and entries) anyway.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
We accomplished this by switching from gevent to cherrypy (should be possible
with gevent, but it wasn't worth the trouble) and shifting the HTTP request
part of the process out of the core TagRss class.
Also added a bit of logging, got rid of /delete_feed returning a spurious
success page in response to GET requests,
|
|
|
|
|
|
|
|
|
|
|
|
|