--db-server validation + e2e tests + review polish

tabcmd/commands/datasources_and_workbooks/publish_command.py

@@ -42,10 +42,27 @@ def define_args(publish_parser):
         set_append_replace_option(group)
         set_parent_project_arg(group)
 
+    @classmethod
+    def _require_db_server_for_workbook(cls, args, filename, logger):
+        """Fail fast if the caller is publishing a workbook with embedded
+        credentials but did not supply --db-server. Called both up front
+        (direct-file case) and inside the per-file publish loop (folder case)."""
+        if (args.db_username or args.oauth_username) and not args.db_server:
+            filename_lower = (filename or "").lower()
+            if filename_lower.endswith(".twb") or filename_lower.endswith(".twbx"):
+                Errors.exit_with_error(logger, _("publish.errors.db_server_required"))
+
     @classmethod
     def run_command(cls, args):
         logger = log(cls.__name__, args.logging_level)
         logger.debug(_("tabcmd.launching"))
+
+        # Fail fast before auth when the direct target is clearly a workbook file:
+        # TSC's workbook publish path requires ConnectionItem.server_address whenever
+        # embedded credentials are supplied. Datasource publishes never need it, so
+        # this check is deliberately scoped to workbook extensions.
+        PublishCommand._require_db_server_for_workbook(args, args.filename, logger)
+
         session = Session()
         server = session.create_session(args, logger)
 
@@ -88,6 +105,9 @@ def run_command(cls, args):
             source = PublishCommand.get_filename_extension_if_tableau_type(logger, str_filename)
             logger.info(_("publish.status").format(str_filename))
             if source in ["twbx", "twb"]:
+                # TSC's workbook publish path requires ConnectionItem.server_address whenever
+                # embedded connection credentials are supplied; datasource publish does not.
+                PublishCommand._require_db_server_for_workbook(args, str_filename, logger)
                 try:
                     published_item = PublishCommand.publish_workbook_file(
                         args=args,
@@ -96,7 +116,7 @@ def run_command(cls, args):
                         project_id=project_id,
                         str_filename=str_filename,
                         publish_mode=publish_mode,
-                        credentials=workbook_connections,
+                        connection=workbook_connections,
                     )
                 except Exception as e:
                     Errors.exit_with_error(logger, exception=e)
@@ -172,7 +192,7 @@ def get_publish_mode(args, logger):
         return publish_mode
 
     @staticmethod
-    def publish_workbook_file(args, logger, server, project_id, str_filename, publish_mode, credentials):
+    def publish_workbook_file(args, logger, server, project_id, str_filename, publish_mode, connection):
         if args.thumbnail_group:
             raise AttributeError("Generating thumbnails for a group is not yet implemented.")
         if args.thumbnail_username and args.thumbnail_group:
@@ -185,7 +205,7 @@ def publish_workbook_file(args, logger, server, project_id, str_filename, publis
             publish_mode,
             # args.thumbnail_username, not yet implemented in tsc
             # args.thumbnail_group,
-            connections=[credentials] if credentials else None,
+            connections=[connection] if connection else None,
             as_job=False,
             skip_connection_check=args.skip_connection_check,
         )

tabcmd/locales/en/tabcmd_messages_en.properties

@@ -75,9 +75,10 @@ logout.short_description=Sign out from the server
 publish.description=Publish a workbook, data source, or extract to the server
 publish.errors.unexpected_server_response=Unexpected response from the server: {0}
 publish.errors.server_resource_not_found=The resource you specified does not exist, or you do not have permission to see it. Check your project name and permissions
+publish.errors.db_server_required=--db-server is required when publishing a workbook with --db-username or --oauth-username. Use --db-server to specify the database server address the embedded credentials should be associated with.
 publish.options.append=Append extract file to existing data source
 publish.options.db-password=Database password for all data sources
-publish.options.db-server=Server address of the database to associate with the embedded connection credentials. Required when using --db-username or --oauth-username to publish a workbook.
+publish.options.db-server=Server address of the database to associate with the embedded connection credentials. Required when using --db-username or --oauth-username to publish a workbook. Must match the connection server stored in the workbook; a mismatch silently drops the embedded credentials.
 publish.options.db-username=Database username for all data sources
 publish.options.encrypt_extracts=Encrypt extracts in the workbook or datasource being published to the server. 
 publish.options.name=Workbook or data source name on the server. If omitted, the workbook or data source will be named after the file name, without the twb(x), tds(x), or tde extension. Publishing a .tde file will create a data source

tabcmd/locales/pt/tabcmd_messages_pt.properties

@@ -77,7 +77,7 @@ publish.errors.unexpected_server_response=Resposta inesperada do servidor: {0}
 publish.errors.server_resource_not_found=O recurso especificado não existe ou você não tem permissão para vê-lo. Verifique o nome e as permissões do projeto
 publish.options.append=Anexe o arquivo de extração à fonte de dados existente
 publish.options.db-password=Senha do banco de dados para todas as fontes de dados
-publish.options.db-server=Endereço do servidor do banco de dado associado as credenciais. Obrigatório ao usar --db-username ou --oauth-username.
+publish.options.db-server=Endereço do servidor do banco de dados associado às credenciais. Obrigatório ao usar --db-username ou --oauth-username.
 publish.options.db-username=Nome de usuário do banco de dados para todas as fontes de dados
 publish.options.encrypt_extracts=Criptografar extrações no servidor
 publish.options.name=O nome da pasta de trabalho/fonte de dados no servidor. Se omitido, a pasta de trabalho/fonte de dados será nomeado de acordo com nome de arquivo, sem a extensão twb(x), tds(x) ou tde. Publicar um arquivo .tde criará uma fonte de dados

tests/commands/test_publish_command.py

@@ -72,7 +72,7 @@ def test_publish_with_creds(self, mock_path, mock_glob, mock_session):
         self.assertEqual(len(connections), 1)
         self.assertEqual(connections[0].server_address, "db.example.com")
 
-    def test_publish_with_creds_no_db_server(self, mock_path, mock_glob, mock_session):
+    def test_publish_with_db_username_missing_db_server_exits(self, mock_path, mock_glob, mock_session):
         set_up_mock_server(mock_session)
         mock_path = set_up_mock_path(mock_path)
 
@@ -87,7 +87,7 @@ def test_publish_with_creds_no_db_server(self, mock_path, mock_glob, mock_sessio
         mock_args.tabbed = True
 
         mock_args.db_username = "username"
-        mock_args.db_password = "oauth_u"
+        mock_args.db_password = "password"
         mock_args.db_server = None
         mock_args.save_db_password = True
         mock_args.oauth_username = None
@@ -97,13 +97,111 @@ def test_publish_with_creds_no_db_server(self, mock_path, mock_glob, mock_sessio
         mock_args.thumbnail_group = None
         mock_args.skip_connection_check = False
 
+        with self.assertRaises(SystemExit):
+            PublishCommand.run_command(mock_args)
+        mock_session.internal_server.workbooks.publish.assert_not_called()
+
+    def test_publish_with_oauth_username_missing_db_server_exits(self, mock_path, mock_glob, mock_session):
+        set_up_mock_server(mock_session)
+        mock_path = set_up_mock_path(mock_path)
+
+        mock_args.overwrite = False
+        mock_args.append = True
+        mock_args.replace = False
+
+        mock_args.filename = "existing_file.twbx"
+        mock_args.project_name = "project-name"
+        mock_args.parent_project_path = "projects"
+        mock_args.name = ""
+        mock_args.tabbed = True
+
+        mock_args.db_username = None
+        mock_args.db_password = None
+        mock_args.db_server = None
+        mock_args.save_db_password = False
+        mock_args.oauth_username = "oauth_user"
+        mock_args.save_oauth = True
+        mock_args.embed = False
+
+        mock_args.thumbnail_username = None
+        mock_args.thumbnail_group = None
+        mock_args.skip_connection_check = False
+
+        with self.assertRaises(SystemExit):
+            PublishCommand.run_command(mock_args)
+        mock_session.internal_server.workbooks.publish.assert_not_called()
+
+    def test_publish_with_oauth_creds(self, mock_path, mock_glob, mock_session):
+        set_up_mock_server(mock_session)
+        mock_path = set_up_mock_path(mock_path)
+
+        mock_args.overwrite = False
+        mock_args.append = True
+        mock_args.replace = False
+
+        mock_args.filename = "existing_file.twbx"
+        mock_args.project_name = "project-name"
+        mock_args.parent_project_path = "projects"
+        mock_args.name = ""
+        mock_args.tabbed = True
+
+        mock_args.db_username = None
+        mock_args.db_password = None
+        mock_args.save_db_password = False
+        mock_args.oauth_username = "oauth_user"
+        mock_args.save_oauth = True
+        mock_args.db_server = "db.example.com"
+        mock_args.embed = False
+
+        mock_args.thumbnail_username = None
+        mock_args.thumbnail_group = None
+        mock_args.skip_connection_check = False
+
         PublishCommand.run_command(mock_args)
         mock_session.internal_server.workbooks.publish.assert_called()
 
         call_kwargs = mock_session.internal_server.workbooks.publish.call_args.kwargs
         connections = call_kwargs["connections"]
         self.assertEqual(len(connections), 1)
-        self.assertIsNone(connections[0].server_address)
+        self.assertEqual(connections[0].server_address, "db.example.com")
+
+    def test_publish_datasource_with_db_username_no_db_server(self, mock_path, mock_glob, mock_session):
+        # Datasource publishes send credentials via _add_credentials_element on the TSC side,
+        # which does not require server_address. --db-server must not be required here.
+        set_up_mock_server(mock_session)
+        # set_up_mock_server only wires .workbooks on the mocked server (TSC.Server class-spec
+        # doesn't expose instance attributes from __init__); wire .datasources by hand.
+        mock_session.internal_server.datasources = mock.MagicMock()
+        mock_path = set_up_mock_path(mock_path)
+        # set_up_mock_path hardcodes splitext -> [_, 'twbx']; override so extension routing
+        # actually sees a datasource here.
+        mock_path.splitext = lambda x: ["file", "tdsx"]
+
+        mock_args.overwrite = False
+        mock_args.append = True
+        mock_args.replace = False
+
+        mock_args.filename = "existing_file.tdsx"
+        mock_args.project_name = "project-name"
+        mock_args.parent_project_path = "projects"
+        mock_args.name = ""
+        mock_args.tabbed = True
+
+        mock_args.db_username = "username"
+        mock_args.db_password = "password"
+        mock_args.db_server = None
+        mock_args.save_db_password = True
+        mock_args.oauth_username = None
+        mock_args.embed = False
+        mock_args.use_tableau_bridge = False
+
+        mock_args.thumbnail_username = None
+        mock_args.thumbnail_group = None
+        mock_args.skip_connection_check = False
+
+        PublishCommand.run_command(mock_args)
+        mock_session.internal_server.datasources.publish.assert_called()
+        mock_session.internal_server.workbooks.publish.assert_not_called()
 
     def test_get_files_to_publish_twbx(self, mock_path, mock_glob, mock_session):
         set_up_mock_server(mock_session)

tests/e2e/online_tests.py

@@ -75,6 +75,8 @@ def get_publishable_name(file_value: str) -> str:
     TDS_FILE_LIVE = "live_mysql.tds"
 
     TWB_FILE_WITH_EMBEDDED_CONNECTION = "EmbeddedCredentials.twb"
+    # server_address baked into the workbook's <connection server=...> element.
+    TWB_FILE_EMBEDDED_CONNECTION_SERVER = "see-internal-slack"
 
     USERS_DETAILS_FILE = "detailed_users.csv"
     USERNAMES_FILE = "usernames.csv"
@@ -142,7 +144,9 @@ def _publish_args(file, name, optional_args=None):
         return arguments
 
     @staticmethod
-    def _publish_creds_args(arguments, db_user=None, db_pass=None, db_save=None, oauth_user=None, oauth_save=None):
+    def _publish_creds_args(
+        arguments, db_user=None, db_pass=None, db_save=None, oauth_user=None, oauth_save=None, db_server=None
+    ):
         if db_user:
             arguments.append("--db-username")
             arguments.append(db_user)
@@ -156,6 +160,9 @@ def _publish_creds_args(arguments, db_user=None, db_pass=None, db_save=None, oau
             arguments.append(oauth_user)
         if oauth_save:
             arguments.append("--save-oauth")
+        if db_server:
+            arguments.append("--db-server")
+            arguments.append(db_server)
         return arguments
 
     @staticmethod
@@ -468,14 +475,50 @@ def test_view_get_png(self):
 
     @pytest.mark.order(11)
     def test_wb_publish_embedded(self):
+        # NOTE: This asserts only that publish did not crash. It does NOT verify
+        # that credentials actually embedded on the server side - a mismatched
+        # --db-server would still exit 0 while silently dropping the creds at
+        # TSC's request-factory boundary. A stronger assertion would populate
+        # connections post-publish and check embed_password=True.
         file = os.path.join("tests", "assets", TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION)
         name_on_server = TestAssets.get_publishable_name(TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION)
         arguments = TabcmdCall._publish_args(file, name_on_server)
-        arguments = TabcmdCall._publish_creds_args(arguments, database_user, database_password, True)
+        arguments = TabcmdCall._publish_creds_args(
+            arguments,
+            database_user,
+            database_password,
+            True,
+            db_server=TestAssets.TWB_FILE_EMBEDDED_CONNECTION_SERVER,
+        )
         arguments.append("--tabbed")
         arguments.append("--skip-connection-check")
         _test_command(arguments)
 
+    @pytest.mark.order(11)
+    def test_wb_publish_embedded_missing_db_server_fails(self):
+        # Publish with --db-username but no --db-server must exit non-zero with our
+        # friendly message rather than a raw tableauserverclient traceback. The guard
+        # short-circuits before any real work, so throwaway credentials are safe here.
+        file = os.path.join("tests", "assets", TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION)
+        name_on_server = TestAssets.get_publishable_name(TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION) + "-no-server"
+        arguments = TabcmdCall._publish_args(file, name_on_server)
+        arguments = TabcmdCall._publish_creds_args(arguments, "placeholder_user", "placeholder_pass", True)
+        arguments.append("--tabbed")
+        arguments.append("--skip-connection-check")
+
+        login_args = setup_e2e.get_login_args()
+        if login_args is None:
+            pytest.skip("No credentials available (credentials.py not found)")
+        calling_args = ["python", "-m", "tabcmd"] + arguments + login_args + [debug_log] + ["--no-certcheck"]
+        result = subprocess.run(calling_args, capture_output=True, text=True)
+
+        assert result.returncode != 0, "expected non-zero exit for missing --db-server"
+        # Localized string OR the raw key (if .mo has not been regenerated yet) both signal our guard.
+        combined = (result.stdout or "") + (result.stderr or "")
+        assert "publish.errors.db_server_required" in combined or "--db-server is required" in combined, (
+            "expected guard message in output; got:\n" + combined
+        )
+
     @pytest.mark.order(12)
     def test_publish_ds(self):
         file = os.path.join("tests", "assets", TestAssets.TDSX_FILE_WITH_EXTRACT)