[patch] Temporary workaround for regression in Tag query functionality

[Steffen Hoffmann]

I've upgraded TagsPlugin to latest trunk version with following devastating effect:

• NotImplementedError on any ticket page - full DoS
• NotImplementedError on any wiki page, as soon as I try to edit - partial DoS

Analysis the provided Traceback led quite straight-forward to disabling the KeywordSuggestPlugin installed there as well. Effect: Full recovery minus keyword suggest functionality, naturally.

This seems to be a regression in Tag query functionality, but as our users really like/need this for everyday work to prevent tag bloat by misspelling/casing-only differences/etc. I've made a quick hack to restore it.

Publishing this is mainly for reference to serve a temporary solution. To fix this inside the TagsPlugin will be the preferred way, but much harder to be done, as the query module there is a different piece of cake, or I've just not tried hard enough.

[Steffen Hoffmann]

quick fix for utilizing Tag query in an insane way, that just works for now

[Steffen Hoffmann]

This issue has been reported as #7857 for TagsPlugin to get it fixed there.

From the perspective of KeywordSuggestPlugin(s maintainer) this might even be seen just as an enhancement request.? Anyway, we should have a solution here while KeywordSuggestPlugin remainings incompatible with TagsPlugin breaking the nearly the whole Trac application.

[Ryan J Ollos]

I'll push this to the trunk along with the patch in #4201.

Sorry this ticket sat here for so long!

[Ryan J Ollos]

While working on applying the patch in #4201 to the trunk, and working with the TagsPlugin trunk @ r10800, prior to applying your patch I was seeing:

File "/home/rjollos/Workspace/th4201/trac-0.11-stable/trac/web/main.py", line 450, in _dispatch_request
  dispatcher.dispatch(req)
File "/home/rjollos/Workspace/th4201/trac-0.11-stable/trac/web/main.py", line 227, in dispatch
  data, content_type)
File "/home/rjollos/Workspace/th4201/trac-0.11-stable/trac/web/chrome.py", line 745, in render_template
  stream |= self._filter_stream(req, method, filename, stream, data)
File "/home/rjollos/Workspace/th4201/genshi-trunk/genshi/core.py", line 132, in __or__
  return Stream(_ensure(function(self)), serializer=self.serializer)
File "/home/rjollos/Workspace/th4201/trac-0.11-stable/trac/web/chrome.py", line 848, in inner
  data)
File "/home/rjollos/Workspace/keywordsuggestplugin/0.11/keywordsuggest/keywordsuggest.py", line 45, in filter_stream
  for resource, tags in query_result:
File "/home/rjollos/Workspace/tagsplugin/trunk/tractags/api.py", line 250, in query
  for m in self._realm.finditer(query.as_string()):
File "/home/rjollos/Workspace/tagsplugin/trunk/tractags/query.py", line 360, in as_string
  return _convert(self)
File "/home/rjollos/Workspace/tagsplugin/trunk/tractags/query.py", line 359, in _convert
  raise NotImplementedError

[Steffen Hoffmann]

Seem, like I've lost track of this ticket.

I think there is a real solution on the way to fix this without my ugly work-around. But do it anyway, as it can easily get reverted when applying a more elaborated fix of the TagsQuery code, as I'm working on #7857.

[Ryan J Ollos]

(In [10935]) Refs #4201, Fixes #7856: Version 0.4: Added support for TagsPlugin. Tags from the TagsPlugin will be used whenever the TagsPlugin is installed. In the future, an option might be added to determine whether or not to use the TagsPlugin when it is installed. The changes here were derived from the source as modified by georgesoon, keywordsuggest.py in #4201.

These changes have been lightly tested, so please open a new ticket if you encounter any issues.

Compatibility note: Previously, the separator option was defined in quotes, e.g. ', '. Now, the separator is just defined as a character, without quotes. A space is added to all separators.

[Steffen Hoffmann]

(In [12392]) TagsPlugin: Make query without arguments a valid query expression, refs #7856 and #7857.

Fixes NotImplementedError meaning a regression, because it has been parsed gracefully before, even got used as valid 'all tags' expression in former plugin versions, and i.e. KeywordSuggestPlugin even relied on this behavior.

Thanks to Itamar Ostricher for contributing the key changes of this changeset.

Some code and template clean-up is done as well.

[Steffen Hoffmann]

So the hot-fix has been made obsolete 1 year later. Not a short fix, but at least done now. Thanks for your patience.

[Steffen Hoffmann]

TagsPlugin now has better way to get queried for existing tags than using TagSystem.query()

I'll attach a patch in a minute to propose changes for this plugin, that replace this work-around for recent versions of TagsPlugin and are recommended especially for performance anyway. See #4503 for details, please.

[Steffen Hoffmann]

Please consider the new proposed fix​ for giving decent performance (~ 45 s less per 5k tickets) on bigger Trac applications.

[Steffen Hoffmann]

I re-opened for keeping focus on the subject, but will create a new ticket, if you prefer that.

[Steffen Hoffmann]

replace query work-around by new, performance-conscious class method

[Ryan J Ollos]

Replying to hasienda:

Please consider the new proposed fix​ for giving decent performance (~ 45 s less per 5k tickets) on bigger Trac applications.

Thanks, please commit it.

Is it correct to say that this will work in TagsPlugin 0.6 and 0.7, (I see athomas added the get_all_tags method way back in [3882]), but the performance will be significantly improved in recent revisions leading up to 0.7?

[Ryan J Ollos]

The autocomplete in TagsPlugin should be used.