`render_resource_link` should add the "missing" class if the resource doesn't exist

[Ryan J Ollos <ryan.j.ollos@…>]

In the case that get_resource_description doesn't return an Element, render_resource_link should check whether the resource exists and add the missing class if it doesn't.

This is mainly useful for plugins at the moment, since render_resource_link is only utilized once in the Trac source. In this one case that it is used, a context parameter is passed, so an Element will be returned by render_resource_link and the pathway that this ticket proposes to modify is not being executed in that case.

I've prepared a patch in ​ff9c10ec. I don't like how the test case adds a dependency on trac.wiki.model.WikiPage, but I didn't have any better ideas at the moment and struggled to figure out the right way to code a "dummy" ResourceManager. I'll iterate on the patch if anyone can provide suggestions on how to improve.

[Ryan J Ollos <ryan.j.ollos@…>]

• ​53e03e02 Added rel='nofollow' attribute to the 'missing link'.

[Ryan J Ollos]

The test cases have been improved. Revised changes proposed in rjollos.git:t11169.2:

• adds the realm as a class for the link.
• adds the missing class and the rel=nofollow attribute to the link if the resource has a ResourceManager and the resource does not exist.

[Ryan J Ollos]

Committed to trunk in [12308].

[Ryan J Ollos]

While implementing this ticket, I noticed a small improvement that might help plugin developers. I was seeing this traceback due to an improper call:

======================================================================
ERROR: test_resource_missing (__main__.RenderResourceLinkTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/Workspace/t11169/teo-rjollos.git/trac/tests/resource.py", line 76, in setUp
    self.resource_manager = FakeResourceManager()
  File "trac/core.py", line 128, in __call__
    compmgr = args[0]
IndexError: tuple index out of range

----------------------------------------------------------------------

Proposed patch is:

trac/core.py

@@ -125 +125 @@
             return self
 
         # The normal case where the component is not also the component manager
+        assert len(args) >= 1 and isinstance(args[0], ComponentManager), \
+               "First argument must be a ComponentManager instance"
         compmgr = args[0]
         self = compmgr.components.get(cls)
         # Note that this check is racy, we intentionally don't use a

trac/tests/core.py

@@ -341 +341 @@
         instance = ComponentA(mgr)
         self.assertIsNone(mgr[ComponentA])
 
+    def test_invalid_argument_raises(self):
+        """
+        AssertionError is raised when first argument to initializer is not a
+        ComponentManager instance.
+        """
+        class ComponentA(Component):
+            pass
+        self.assertRaises(AssertionError, Component)
+
 
 def suite():
     return unittest.makeSuite(ComponentTestCase, 'test')

[Ryan J Ollos]

There were a few errors when running the full test suite, such as:

======================================================================
FAIL: test_explicit_failure (__main__.WithTransactionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/Workspace/t11390/teo-rjollos.git/trac/db/tests/api.py", line 122, in test_explicit_failure
    @with_transaction(env, db)
  File "trac/db/api.py", line 77, in with_transaction
    dbm = DatabaseManager(env)
  File "trac/core.py", line 129, in __call__
    "First argument must be a ComponentManager instance"
AssertionError: First argument must be a ComponentManager instance

I've improved the patch in log:rjollos.git:t11169.3 to avoid the test failures. Does it look okay?

[Jun Omae]

LGTM.

[Ryan J Ollos]

Thanks for reviewing. Committed to 1.0-stable in [12311]; merged to trunk in [12312].