[Patch] Exclude PageTemplates from tag-extraction

[Itamar Ostricher]

Ticket #3753 brought a long-awaited feature - propagation of tags from PageTemplates to the created page.

Unfortunately, the side effect of the selected solution is that TagsPlugin treats the pages under PageTemplates as any other tagged wiki resource, which means they are included in the tags cloud as well as in results of ListTagged macro.

At least for me, this is undesired, since the templates are "meta-content", and not actual content that I want to be treated the same.

My request is to add an option to exclude everything under PageTemplates from tags extraction.

[Steffen Hoffmann]

Good catch, just what I noticed recently too.

+1 for fixing this. Suggestions, patch? :-)

[Itamar Ostricher]

patch against tagsplugin trunk

[Itamar Ostricher]

Replying to hasienda:

Good catch, just what I noticed recently too.

+1 for fixing this. Suggestions, patch? :-)

You ask so nicely, so I could not resist :-)

Patch attached​, maybe somewhat dirty, as I add in api a specific test for the wiki realm, but it works for me :-)

On the way - the patch contains a fix for a possible issue my IDE warned me about (resource not defined in the context of the PermissionError exception).

[Steffen Hoffmann]

While your fix for the undefined variable resource seems perfectly valid, I wonder how to alter the suggested change for the main issue.

Intercepting get_tagged_resources seems fine only on first look. But it would mask wiki templates for the admin tag replace feature too, and I seriously doubt that's desirable or one of your explicit intentions.

[Itamar Ostricher]

attempting to fix the tag-replace issue with previous patch

[Itamar Ostricher]

Replying to hasienda:

Intercepting get_tagged_resources seems fine only on first look. But it would mask wiki templates for the admin tag replace feature too, and I seriously doubt that's desirable or one of your explicit intentions.

You're absolutely right. A second version​ of the patch tries to resolve this (quite a hack I guess, but couldn't think of a better approach).

[Steffen Hoffmann]

Very well. That should work indeed.

Reason, why I don't apply this right-away is, that after starting to work on #4503 I've never let performance go out of sight. I guess that not retrieving data is better than filtering it out later, so I tend towards implementing a more sophisticated SQL statement inside get_tagged_resources that could be fed by a new optional filter argument. Just finalizing and testing this right now ...

[anonymous]

Great. Good thing you're keeping an eye on performance - not quite my strong side :-)

That said, I'm not completely sure "more sophisticated SQL" to prevent data retrieval would be better performance-wise compared to dead-simple SQL with simple filtering out.

Using more sophisticated SQL just passes on the bottleneck the the SQL library, so it's not explicit.

Also I generally prefer readability and maintainability over performance when the effect is not considerable.

But, as I said - I don't know much about these things. Just my 2c.

[Steffen Hoffmann]

(In [11105]) TagsPlugin: Fix undefined resource in PermissionError exception context, refs #9636.

This has been reported and corrected in KISS-style by Itamar Ostricher.

[Adrian Fritz]

Just to point a work-around:

1. add another Tag like "PageTemplate" and
2. then exclude it when using [[ListTagged(TagOfInterest1 TagOfInterest2 -PageTemplate)]] macro.

[Sam Halliday]

But hold on, if you add a tag to the PageTemplate then it will be propagated to new pages and therefore those pages will be excluded.

[Steffen Hoffmann]

Replying to fommil:

But hold on, if you add a tag to the PageTemplate then it will be propagated to new pages and therefore those pages will be excluded.

Indeed. Still I don't see a reason, why we shouldn't implement this in a way, that is usable for Trac-0.11 onwards, not just 1.0, right?

[Steffen Hoffmann]

Replying to anonymous:

That said, I'm not completely sure "more sophisticated SQL" to prevent data retrieval would be better performance-wise compared to dead-simple SQL with simple filtering out. Using more sophisticated SQL just passes on the bottleneck the the SQL library, so it's not explicit.

I think I've found a valid answer testing with 30.000 auto-generated tickets. On my development system it took around 100-300 ms to pull the keywords from all tickets, but more than 3 seconds to check access permission on each one. Well, maybe a good share of the time has been constructing the Trac Resource objects for the check, but it remains questionable to me, if there is such a thing like "simple filtering". Only by using rather heavy recursion in the SQL statement I could imagine to hit db performance to the point, where post processing in Python could gain speed.

Also I generally prefer readability and maintainability over performance when the effect is not considerable.

The aforementioned Test environment takes 15 s or longer to render an all-tagged-resources list of these 30.000 tickets. It still takes about 6 s for each tag cloud render pass. Scaling this up to 500.000+ tickets know to sit in some other, real-world Trac applications, there is definitely a big point for performance. I've learned this and will not sleep relaxed before we've gained at least ~ 10 times of the current speed.

[Steffen Hoffmann]

(In [12390]) TagsPlugin: Don't use tags on wiki templates for anything else than copying tags, refs #3753 and #9636.

Once again this has been reported by Itamar Ostricher. Thank you very much for nudging me to fix it.

[Adrian Fritz]

I deduced from digging into source code, tags in wiki pages located under specific "sud-dir" will not be "tagged" anymore. Example:

• from source def get_tagged_resources(req, tags=None, filter=None)
  • setting under [PageTemplates] section in trac.ini file filter = PageTemplates

Am I correct?

[Adrian Fritz]

Replying to fommil:

But hold on, if you add a tag to the PageTemplate then it will be propagated to new pages and therefore those pages will be excluded.

Yes, it propagates PageTemplates too. But still very handy (in our use case at least ).

[Steffen Hoffmann]

Replying to AdrianFritz:

I deduced from digging into source code, tags in wiki pages located under specific "sud-dir" will not be "tagged" anymore. Example:

• from source def get_tagged_resources(req, tags=None, filter=None)
  • setting under [PageTemplates] section in trac.ini file filter = PageTemplates

Am I correct?

Yes, tags on any page below PageTemplates (technically correct: sharing common path 'PageTemplates/') are excluded from 'all-realm' or 'all-Trac' tag lists, but a per-resource query still shows them, as does the display of the wiki page view or a corresponding field in wiki page editor.

[Steffen Hoffmann]

Replying to fommil:

But hold on, if you add a tag to the PageTemplate then it will be propagated to new pages and therefore those pages will be excluded.

Yes, so I felt urged to implement the better approach: The new (implicit) default configuration keeps tag lists working and in-depended of such meta-tag hacks, that require user action to not exclude new pages from tag lists derived from the respective template as well.

[Steffen Hoffmann]

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.