Modify

Opened 4 years ago

Closed 2 weeks ago

#7857 closed defect (fixed)

[PATCH] Regression in Tag query functionality

Reported by: hasienda Owned by: hasienda
Priority: high Component: TagsPlugin
Severity: normal Keywords: regression
Cc: rjollos, itamarost Trac Release: 0.11

Description

Empty query as in

[[ListTagged()]]

raises NotImplementedError

This has been most relevant in some applications, that I upgraded today from v0.6 to recent trunk with devastating effect on the whole environments. In short, I had to temporarily disable KeywordSuggestPlugin installed there as well, and only later found a quick but ugly hack, to get it working again (see #7856).

It looks like KeywordSuggestPlugin has been built on a default query behavior to return all known tags, that got lost in later versions of TagsPlugin. So it should be fixed here, or I totally mistake the situation.

Attachments (3)

tagsplugin-7857-itamaro-v1.patch (538 bytes) - added by itamarost 3 years ago.
patch against tagsplugin trunk
ListTaggedEmptyCall-r10708.png (7.0 KB) - added by rjollos 3 years ago.
tagsplugin-7857-itamaro-v2.patch (1.7 KB) - added by itamarost 3 years ago.
some more tweaks…

Download all attachments as: .zip

Change History (20)

comment:1 follow-up: Changed 3 years ago by shentonfreude

I was bitten by this as well, also using the KeywordSuggestPlugin with the autocomplete patch. I ended up backing out Tags to the 0.6 version.

I'm using Trac-0.12.

comment:2 in reply to: ↑ 1 Changed 3 years ago by hasienda

  • Owner changed from athomas to otaku42

Replying to shentonfreude:

I was bitten by this as well, also using the KeywordSuggestPlugin with the autocomplete patch. ![...]

Thanks for the confirmation.

However, knowing the issue is there is just the first step. Did you investigate the offending change(s) in TagsPlugin by now? I'd love to help fix this, but have not enough time yet to research it on my own.

Don't hesitate to propose a patch against trunk. I'm well able to review and commit, if suitable.

Changed 3 years ago by itamarost

patch against tagsplugin trunk

comment:3 Changed 3 years ago by itamarost

  • Cc itamarost added
  • Summary changed from Regression in Tag query funcionality to [PATCH] Regression in Tag query funcionality

I don't fully understand the underlying issue, but the attached patch fixed this for me.

Changed 3 years ago by rjollos

comment:4 follow-up: Changed 3 years ago by rjollos

On Trac 0.11.7, with the latest version of the TagsPlugin trunk (r10708), [[ListTagged()]] gives me:


The traceback is:

2011-09-29 19:17:30,662 Trac[formatter] ERROR: Macro ListTagged() failed:
Traceback (most recent call last):
  File "/usr/lib/python2.6/site-packages/Trac-0.11.7-py2.6.egg/trac/wiki/formatter.py", line 484, in _macro_formatter
    return macro.process(args, in_paragraph=True)
  File "/usr/lib/python2.6/site-packages/Trac-0.11.7-py2.6.egg/trac/wiki/formatter.py", line 180, in process
    text = self.processor(text)
  File "/usr/lib/python2.6/site-packages/Trac-0.11.7-py2.6.egg/trac/wiki/formatter.py", line 167, in _macro_processor
    text)
  File "/usr/lib/python2.6/site-packages/TracTags-0.7dev_r10708-py2.6.egg/tractags/macros.py", line 173, in expand_macro
    if len(realms) == 0:
TypeError: object of type 'NoneType' has no len()

comment:5 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by hasienda

Replying to rjollos:

On Trac 0.11.7, with the latest version of the TagsPlugin trunk (r10708), [[ListTagged()]] gives me:

...
TypeError: object of type 'NoneType' has no len()

This latest observation is with source patched with Itamar's patch, or not? I've not seen this error, just the old one. But I'll try to fix this parser issue before tractags-0.7 for sure.

comment:6 in reply to: ↑ 5 Changed 3 years ago by rjollos

Replying to hasienda:

This latest observation is with source patched with Itamar's patch, or not?

Observation is with a clean version of r10708; no patch.

comment:7 Changed 3 years ago by osimons

Make a unit test, perhaps?

Changed 3 years ago by itamarost

some more tweaks...

comment:8 Changed 3 years ago by itamarost

Since updating to r10708, on 0.12.3dev-r10799, with my previous patch, I ran into some more issues (including the one mentioned by rjollos above), and the v2 of the patch helps a little, although it seems there are still deeper issues (indeed - some unit tests are appropriate).

As it is now, with the v2 patch, I don't get errors, but there's something weird with the Query object.
I used the listtagged_exclude_realms = ticket option and created several tagged wiki pages and tickets (lets say the tag is bla for all).

  • If I call [[ListTagged(bla)]] I get the expected result - a list of only wiki pages with that tag.
  • If I call [[ListTagged()]] I get the unexpected result of wiki AND tickets.
  • If I specify a realm explicitly but with no tags, e.g. [[ListTagged(realm:wiki)]] or [[ListTagged(realm:ticket)]] I do get the expected result.

Trying to debug it, I noticed that in all cases, in exapnd_macro() the query looks OK when it gets to query_result = tag_system.query(req, query):

  • [[ListTagged(bla)]] --> query = '(bla) (realm:wiki)'
  • [[ListTagged()]] --> query = '() (realm:wiki)'

But the Query object gets messed up somehow in the process...

comment:9 Changed 3 years ago by osimons

(In [10714]) TagsPlugin: Fix TypeError: object of type 'NoneType' has no len() for [[ListTagged()]] with test. References #7857.

comment:10 Changed 3 years ago by osimons

Just wanted to get the macro tests going, and fixing the error message seemed like a good start. As I don't actually use the plugin, I'll leave the other issues and tests for others...

comment:11 Changed 18 months ago by rjollos

  • Summary changed from [PATCH] Regression in Tag query funcionality to [PATCH] Regression in Tag query functionality

comment:12 Changed 17 months ago by hasienda

  • Owner changed from otaku42 to hasienda
  • Priority changed from normal to high

As far as I traced it down, this should probably be fixed inside the query parser, but I've been unable to finally fix it for many months now.

Confirming that the fix will become even more important to me within the next release cycle. I would certainly consider a maintenance release after getting it done. Sorry for the delay.

comment:13 Changed 17 months ago by hasienda

(In [12391]) TagsPlugin: Make realm configuration for ListTagged more effective, refs #7857 and #9063.

Settings for 'listtagged_exclude_realms' had no effect without a query.
Itamar Ostricher discovered this issue in the initial implementation of the
feature and suggested this change. Thanks for taking care.

Additionally the broken 'test_empty_content' test in
tractags.tests.macros.ListTaggedMacroTestCase is fixed, and a regression test
for the pending 'empty query' issue (#7857) has been introduced as well.

comment:14 Changed 17 months ago by hasienda

(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.

comment:15 follow-up: Changed 17 months ago by hasienda

All 34 tests pass for all supported configurations including Trac versions ranging from 0.11 to 1.0, tested with the PostgreSQL db backend as well - what a relief.

comment:16 in reply to: ↑ 15 Changed 17 months ago by itamarost

Replying to hasienda:

All 34 tests pass for all supported configurations including Trac versions ranging from 0.11 to 1.0, tested with the PostgreSQL db backend as well - what a relief.

That's awesome!
I'm glad you were able to get this into the plugin :-)

Hope I'll find an opportunity to test this in my environments, though I don't see one in the near future...

comment:17 Changed 2 weeks ago by hasienda

  • Resolution set to fixed
  • Status changed from new to closed

In 13815:

TagsPlugin: Completing preparation for v0.7 release.

Availability of that code as stable, tagged release
closes #2429, #3359, #3610, #3624, #3677, #3754, #3864, #3947, #3983, #4078, #4277, #4503, #4799, #5523, #7787, #7857, #8638, #9057, #9058, #9059, #9060, #9061, #9062, #9063, #9149, #9210, #9521, #9630, #9636, #10032, #10416, #10636, #11096, #11147, #11152, #11274, #11302, #11658 and #11659.

Additionally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily, specifically
refs #2804, #4200, #8747 and #9064.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.