feat: check all user specified spaces for confluence source (#523)

Improve `precheck` method of Confluence's Indexer.

Validate that each space provided in configuration can be accessed, raise exception if at least one of them can't.

---------

Co-authored-by: Rob Roskam <[email protected]>
Co-authored-by: Filip Knefel <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.0.34-dev
+
+* **Improve Confluence Indexer's precheck** - validate access to each space
+
 ## 1.0.33
 
 * **Fix google drive not setting the display_name property on the FileData object**

test/unit/connectors/test_confluence.py

@@ -1,12 +1,27 @@
+from unittest import mock
+
 import pytest
 from pydantic import ValidationError
 
 from unstructured_ingest.processes.connectors.confluence import (
     ConfluenceAccessConfig,
     ConfluenceConnectionConfig,
+    ConfluenceIndexer,
+    ConfluenceIndexerConfig,
 )
 
 
+@pytest.fixture
+def connection_config():
+    """Provides a minimal ConfluenceConnectionConfig for testing."""
+    access_config = ConfluenceAccessConfig(api_token="token")
+    return ConfluenceConnectionConfig(
+        url="https://dummy",
+        username="user",
+        access_config=access_config,
+    )
+
+
 def test_connection_config_multiple_auth():
     with pytest.raises(ValidationError):
         ConfluenceConnectionConfig(
@@ -69,3 +84,80 @@ def test_connection_config_pat_auth():
         access_config=ConfluenceAccessConfig(token="access_token"),
         url="url",
     )
+
+
+def test_precheck_with_spaces_calls_get_space(monkeypatch, connection_config):
+    """Test that precheck calls get_space for each space when spaces are set."""
+    spaces = ["A", "B", "C"]
+    index_config = ConfluenceIndexerConfig(
+        max_num_of_spaces=100,
+        max_num_of_docs_from_each_space=100,
+        spaces=spaces,
+    )
+    indexer = ConfluenceIndexer(connection_config=connection_config, index_config=index_config)
+    mock_client = mock.MagicMock()
+    with mock.patch.object(type(connection_config), "get_client", mock.MagicMock()):
+        type(connection_config).get_client.return_value.__enter__.return_value = mock_client
+
+        result = indexer.precheck()
+        calls = [mock.call(space) for space in spaces]
+        mock_client.get_space.assert_has_calls(calls, any_order=False)
+        assert mock_client.get_space.call_count == len(spaces)
+        assert result is True
+
+
+def test_precheck_without_spaces_calls_get_all_spaces(monkeypatch, connection_config):
+    """Test that precheck calls get_all_spaces when spaces is not set."""
+    index_config = ConfluenceIndexerConfig(
+        max_num_of_spaces=100,
+        max_num_of_docs_from_each_space=100,
+        spaces=None,
+    )
+    indexer = ConfluenceIndexer(connection_config=connection_config, index_config=index_config)
+    mock_client = mock.MagicMock()
+    with mock.patch.object(type(connection_config), "get_client", mock.MagicMock()):
+        type(connection_config).get_client.return_value.__enter__.return_value = mock_client
+
+        result = indexer.precheck()
+        mock_client.get_all_spaces.assert_called_once_with(limit=1)
+        mock_client.get_space.assert_not_called()
+        assert result is True
+
+
+def test_precheck_with_spaces_raises(monkeypatch, connection_config):
+    """Test that precheck raises UserError if get_space fails."""
+    spaces = ["A", "B"]
+    index_config = ConfluenceIndexerConfig(
+        max_num_of_spaces=100,
+        max_num_of_docs_from_each_space=100,
+        spaces=spaces,
+    )
+    indexer = ConfluenceIndexer(connection_config=connection_config, index_config=index_config)
+    mock_client = mock.MagicMock()
+    mock_client.get_space.side_effect = Exception("fail")
+    from unstructured_ingest.processes.connectors.confluence import UserError
+
+    with mock.patch.object(type(connection_config), "get_client", mock.MagicMock()):
+        type(connection_config).get_client.return_value.__enter__.return_value = mock_client
+
+        with pytest.raises(UserError):
+            indexer.precheck()
+
+
+def test_precheck_without_spaces_raises(monkeypatch, connection_config):
+    """Test that precheck raises SourceConnectionError if get_all_spaces fails."""
+    index_config = ConfluenceIndexerConfig(
+        max_num_of_spaces=100,
+        max_num_of_docs_from_each_space=100,
+        spaces=None,
+    )
+    indexer = ConfluenceIndexer(connection_config=connection_config, index_config=index_config)
+    mock_client = mock.MagicMock()
+    mock_client.get_all_spaces.side_effect = Exception("fail")
+    from unstructured_ingest.processes.connectors.confluence import UserError
+
+    with mock.patch.object(type(connection_config), "get_client", mock.MagicMock()):
+        type(connection_config).get_client.return_value.__enter__.return_value = mock_client
+
+        with pytest.raises(UserError):
+            indexer.precheck()

unstructured_ingest/__version__.py

@@ -1 +1 @@
-__version__ = "1.0.33"  # pragma: no cover
+__version__ = "1.0.34-dev"  # pragma: no cover

unstructured_ingest/processes/connectors/confluence.py

@@ -12,6 +12,7 @@
     SourceIdentifiers,
 )
 from unstructured_ingest.error import SourceConnectionError
+from unstructured_ingest.errors_v2 import UserAuthError, UserError
 from unstructured_ingest.interfaces import (
     AccessConfig,
     ConnectionConfig,
@@ -96,7 +97,7 @@ def password_or_api_token(self) -> str:
 
     @requires_dependencies(["atlassian"], extras="confluence")
     @contextmanager
-    def get_client(self) -> "Confluence":
+    def get_client(self) -> Generator["Confluence", None, None]:
         from atlassian import Confluence
 
         access_configs = self.access_config.get_secret_value()
@@ -126,15 +127,36 @@ class ConfluenceIndexer(Indexer):
 
     def precheck(self) -> bool:
         try:
-            # Attempt to retrieve a list of spaces with limit=1.
-            # This should only succeed if all creds are valid
-            with self.connection_config.get_client() as client:
+            self.connection_config.get_client()
+        except Exception as e:
+            logger.exception(f"Failed to connect to Confluence: {e}")
+            raise UserAuthError(f"Failed to connect to Confluence: {e}")
+
+        with self.connection_config.get_client() as client:
+            # opportunistically check the first space in list of all spaces
+            try:
                 client.get_all_spaces(limit=1)
+            except Exception as e:
+                logger.exception(f"Failed to connect to find any Confluence space: {e}")
+                raise UserError(f"Failed to connect to find any Confluence space: {e}")
+
             logger.info("Connection to Confluence successful.")
-            return True
-        except Exception as e:
-            logger.error(f"Failed to connect to Confluence: {e}", exc_info=True)
-            raise SourceConnectionError(f"Failed to connect to Confluence: {e}")
+
+            # If specific spaces are provided, check if we can access them
+            errors = []
+
+            if self.index_config.spaces:
+                for space_key in self.index_config.spaces:
+                    try:
+                        client.get_space(space_key)
+                    except Exception as e:
+                        logger.exception(f"Failed to connect to Confluence: {e}")
+                        errors.append(f"Failed to connect to '{space_key}' space, cause: '{e}'")
+
+            if errors:
+                raise UserError("\n".join(errors))
+
+        return True
 
     def _get_space_ids_and_keys(self) -> List[Tuple[str, int]]:
         """
@@ -406,7 +428,7 @@ def run(self, file_data: FileData, **kwargs) -> download_responses:
                     expand="history.lastUpdated,version,body.view",
                 )
         except Exception as e:
-            logger.error(f"Failed to retrieve page with ID {doc_id}: {e}", exc_info=True)
+            logger.exception(f"Failed to retrieve page with ID {doc_id}: {e}")
             raise SourceConnectionError(f"Failed to retrieve page with ID {doc_id}: {e}")
 
         if not page: