fix: eliminate circular reference in Confluence preprocessor (sooperset#574)

* fix: eliminate circular reference in Confluence preprocessor

Remove circular reference between ConfluenceClient and ConfluencePreprocessor
that was preventing proper garbage collection and causing memory leaks.

The preprocessor no longer stores a reference to the client. Instead, the
client is passed as a parameter to methods that need it for user lookups.
This follows the same pattern used by JiraPreprocessor.

* fix: update test assertions to include confluence client parameter

Author: sooperset

src/mcp_atlassian/confluence/client.py

@@ -121,9 +121,7 @@ def __init__(self, config: ConfluenceConfig | None = None) -> None:
         # Import here to avoid circular imports
         from ..preprocessing.confluence import ConfluencePreprocessor
 
-        self.preprocessor = ConfluencePreprocessor(
-            base_url=self.config.url, confluence_client=self.confluence
-        )
+        self.preprocessor = ConfluencePreprocessor(base_url=self.config.url)
 
         # Test authentication during initialization (in debug mode only)
         if logger.isEnabledFor(logging.DEBUG):
@@ -186,4 +184,6 @@ def _process_html_content(
         Returns:
             Tuple of (processed_html, processed_markdown)
         """
-        return self.preprocessor.process_html_content(html_content, space_key)
+        return self.preprocessor.process_html_content(
+            html_content, space_key, self.confluence
+        )

src/mcp_atlassian/confluence/comments.py

@@ -43,7 +43,9 @@ def get_page_comments(
                 # Get the content based on format
                 body = comment_data["body"]["view"]["value"]
                 processed_html, processed_markdown = (
-                    self.preprocessor.process_html_content(body, space_key=space_key)
+                    self.preprocessor.process_html_content(
+                        body, space_key=space_key, confluence_client=self.confluence
+                    )
                 )
 
                 # Create a copy of the comment data to modify
@@ -117,6 +119,7 @@ def add_comment(self, page_id: str, content: str) -> ConfluenceComment | None:
             processed_html, processed_markdown = self.preprocessor.process_html_content(
                 response.get("body", {}).get("view", {}).get("value", ""),
                 space_key=space_key,
+                confluence_client=self.confluence,
             )
 
             # Modify the response to include processed content

src/mcp_atlassian/confluence/pages.py

@@ -54,7 +54,7 @@ def get_page_content(
             space_key = page.get("space", {}).get("key", "")
             content = page["body"]["storage"]["value"]
             processed_html, processed_markdown = self.preprocessor.process_html_content(
-                content, space_key=space_key
+                content, space_key=space_key, confluence_client=self.confluence
             )
 
             # Use the appropriate content format based on the convert_to_markdown flag
@@ -169,7 +169,7 @@ def get_page_by_title(
 
             content = page["body"]["storage"]["value"]
             processed_html, processed_markdown = self.preprocessor.process_html_content(
-                content, space_key=space_key
+                content, space_key=space_key, confluence_client=self.confluence
             )
 
             # Use the appropriate content format based on the convert_to_markdown flag
@@ -230,7 +230,7 @@ def get_space_pages(
         for page in pages:
             content = page["body"]["storage"]["value"]
             processed_html, processed_markdown = self.preprocessor.process_html_content(
-                content, space_key=space_key
+                content, space_key=space_key, confluence_client=self.confluence
             )
 
             # Use the appropriate content format based on the convert_to_markdown flag
@@ -477,7 +477,9 @@ def get_page_children(
                     content = page.get("body", {}).get("storage", {}).get("value", "")
                     if content:
                         _, processed_markdown = self.preprocessor.process_html_content(
-                            content, space_key=space_key
+                            content,
+                            space_key=space_key,
+                            confluence_client=self.confluence,
                         )
                         content_override = processed_markdown
 

src/mcp_atlassian/confluence/search.py

@@ -82,7 +82,9 @@ def search(
                         # Process the excerpt as HTML content
                         space_key = page.space.key if page.space else ""
                         _, processed_markdown = self.preprocessor.process_html_content(
-                            excerpt, space_key=space_key
+                            excerpt,
+                            space_key=space_key,
+                            confluence_client=self.confluence,
                         )
                         # Create a new page with processed content
                         page.content = processed_markdown

src/mcp_atlassian/preprocessing/base.py

@@ -26,28 +26,28 @@ def get_user_details_by_username(self, username: str) -> dict[str, Any]:
 class BasePreprocessor:
     """Base class for text preprocessing operations."""
 
-    def __init__(
-        self, base_url: str = "", confluence_client: ConfluenceClient | None = None
-    ) -> None:
+    def __init__(self, base_url: str = "") -> None:
         """
         Initialize the base text preprocessor.
 
         Args:
             base_url: Base URL for API server
-            confluence_client: Optional Confluence client for user lookups
         """
         self.base_url = base_url.rstrip("/") if base_url else ""
-        self.confluence_client = confluence_client
 
     def process_html_content(
-        self, html_content: str, space_key: str = ""
+        self,
+        html_content: str,
+        space_key: str = "",
+        confluence_client: ConfluenceClient | None = None,
     ) -> tuple[str, str]:
         """
         Process HTML content to replace user refs and page links.
 
         Args:
             html_content: The HTML content to process
             space_key: Optional space key for context
+            confluence_client: Optional Confluence client for user lookups
 
         Returns:
             Tuple of (processed_html, processed_markdown)
@@ -57,8 +57,8 @@ def process_html_content(
             soup = BeautifulSoup(html_content, "html.parser")
 
             # Process user mentions
-            self._process_user_mentions_in_soup(soup)
-            self._process_user_profile_macros_in_soup(soup)
+            self._process_user_mentions_in_soup(soup, confluence_client)
+            self._process_user_profile_macros_in_soup(soup, confluence_client)
 
             # Convert to string and markdown
             processed_html = str(soup)
@@ -70,12 +70,15 @@ def process_html_content(
             logger.error(f"Error in process_html_content: {str(e)}")
             raise
 
-    def _process_user_mentions_in_soup(self, soup: BeautifulSoup) -> None:
+    def _process_user_mentions_in_soup(
+        self, soup: BeautifulSoup, confluence_client: ConfluenceClient | None = None
+    ) -> None:
         """
         Process user mentions in BeautifulSoup object.
 
         Args:
             soup: BeautifulSoup object containing HTML
+            confluence_client: Optional Confluence client for user lookups
         """
         # Find all ac:link elements that might contain user mentions
         user_mentions = soup.find_all("ac:link")
@@ -86,7 +89,9 @@ def _process_user_mentions_in_soup(self, soup: BeautifulSoup) -> None:
                 # Case 1: Direct user reference without link-body
                 account_id = user_ref.get("ri:account-id")
                 if isinstance(account_id, str):
-                    self._replace_user_mention(user_element, account_id)
+                    self._replace_user_mention(
+                        user_element, account_id, confluence_client
+                    )
                     continue
 
             # Case 2: User reference with link-body containing @
@@ -96,16 +101,21 @@ def _process_user_mentions_in_soup(self, soup: BeautifulSoup) -> None:
                 if user_ref and user_ref.get("ri:account-id"):
                     account_id = user_ref.get("ri:account-id")
                     if isinstance(account_id, str):
-                        self._replace_user_mention(user_element, account_id)
+                        self._replace_user_mention(
+                            user_element, account_id, confluence_client
+                        )
 
-    def _process_user_profile_macros_in_soup(self, soup: BeautifulSoup) -> None:
+    def _process_user_profile_macros_in_soup(
+        self, soup: BeautifulSoup, confluence_client: ConfluenceClient | None = None
+    ) -> None:
         """
         Process Confluence User Profile macros in BeautifulSoup object.
         Replaces <ac:structured-macro ac:name="profile">...</ac:structured-macro>
         with the user's display name, typically formatted as @DisplayName.
 
         Args:
             soup: BeautifulSoup object containing HTML
+            confluence_client: Optional Confluence client for user lookups
         """
         profile_macros = soup.find_all(
             "ac:structured-macro", attrs={"ac:name": "profile"}
@@ -134,26 +144,24 @@ def _process_user_profile_macros_in_soup(self, soup: BeautifulSoup) -> None:
             user_identifier_for_log = account_id or userkey
             display_name = None
 
-            if self.confluence_client and user_identifier_for_log:
+            if confluence_client and user_identifier_for_log:
                 try:
                     if account_id and isinstance(account_id, str):
-                        user_details = (
-                            self.confluence_client.get_user_details_by_accountid(
-                                account_id
-                            )
+                        user_details = confluence_client.get_user_details_by_accountid(
+                            account_id
                         )
                         display_name = user_details.get("displayName")
                     elif userkey and isinstance(userkey, str):
                         # For Confluence Server/DC, userkey might be the username
-                        user_details = (
-                            self.confluence_client.get_user_details_by_username(userkey)
+                        user_details = confluence_client.get_user_details_by_username(
+                            userkey
                         )
                         display_name = user_details.get("displayName")
                 except Exception as e:
                     logger.warning(
                         f"Error fetching user details for profile macro (user: {user_identifier_for_log}): {e}"
                     )
-            elif not self.confluence_client:
+            elif not confluence_client:
                 logger.warning(
                     "Confluence client not available for User Profile Macro processing."
                 )
@@ -171,18 +179,24 @@ def _process_user_profile_macros_in_soup(self, soup: BeautifulSoup) -> None:
                 macro_element.replace_with(fallback_text)
                 logger.debug(f"Using fallback for user profile macro: {fallback_text}")
 
-    def _replace_user_mention(self, user_element: Tag, account_id: str) -> None:
+    def _replace_user_mention(
+        self,
+        user_element: Tag,
+        account_id: str,
+        confluence_client: ConfluenceClient | None = None,
+    ) -> None:
         """
         Replace a user mention with the user's display name.
 
         Args:
             user_element: The HTML element containing the user mention
             account_id: The user's account ID
+            confluence_client: Optional Confluence client for user lookups
         """
         try:
             # Only attempt to get user details if we have a valid confluence client
-            if self.confluence_client is not None:
-                user_details = self.confluence_client.get_user_details_by_accountid(
+            if confluence_client is not None:
+                user_details = confluence_client.get_user_details_by_accountid(
                     account_id
                 )
                 display_name = user_details.get("displayName", "")

src/mcp_atlassian/preprocessing/confluence.py

@@ -4,7 +4,6 @@
 import shutil
 import tempfile
 from pathlib import Path
-from typing import Any
 
 from md2conf.converter import (
     ConfluenceConverterOptions,
@@ -22,15 +21,14 @@
 class ConfluencePreprocessor(BasePreprocessor):
     """Handles text preprocessing for Confluence content."""
 
-    def __init__(self, base_url: str, **kwargs: Any) -> None:
+    def __init__(self, base_url: str) -> None:
         """
         Initialize the Confluence text preprocessor.
 
         Args:
             base_url: Base URL for Confluence API
-            **kwargs: Additional arguments for the base class
         """
-        super().__init__(base_url=base_url, **kwargs)
+        super().__init__(base_url=base_url)
 
     def markdown_to_confluence_storage(
         self, markdown_content: str, *, enable_heading_anchors: bool = False