diff --git a/pyproject.toml b/pyproject.toml index c06f7685..82b20902 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,7 @@ dependencies = [ "appdirs", "requests>=2.25,<3.0", "setuptools", - "tableauserverclient==0.41", + "tableauserverclient==0.42", "urllib3>=2.6.0,<3", ] [project.optional-dependencies] diff --git a/tabcmd/commands/user/create_site_users.py b/tabcmd/commands/user/create_site_users.py index c63436b0..f30f7d36 100644 --- a/tabcmd/commands/user/create_site_users.py +++ b/tabcmd/commands/user/create_site_users.py @@ -1,4 +1,5 @@ import tableauserverclient as TSC +from tableauserverclient.server.endpoint.exceptions import JobFailedException from tabcmd.commands.auth.session import Session from tabcmd.commands.constants import Errors @@ -25,6 +26,9 @@ def define_args(create_site_users_parser): set_users_file_positional(args_group) set_completeness_options(args_group) UserCommand.set_auth_arg(args_group) + set_no_wait_option(args_group) + set_silent_option(args_group) + args_group.add_argument("--errors-file", default=None, help=_("createsiteusers.options.errors-file")) @classmethod def run_command(cls, args): @@ -32,42 +36,144 @@ def run_command(cls, args): logger.debug(_("tabcmd.launching")) session = Session() server = session.create_session(args, logger) - number_of_users_listed = 0 - number_of_users_added = 0 - number_of_errors = 0 creation_site = "current site" + # Pre-flight validation. Under --complete (default) any CSV-shape error + # aborts the whole run before submission -- matching Classic's + # with_transaction semantics as closely as we can without server support. + # Under --no-complete we validate leniently and let the server sort out + # remaining issues per-row. UserCommand.validate_file_for_import(args.filename, logger, detailed=True, strict=args.require_all_valid) - logger.info(_("addusers.status").format(args.filename.name, creation_site)) + if not args.silent_progress: + logger.info(_("addusers.status").format(args.filename.name, creation_site)) + user_obj_list = UserCommand.get_users_from_file(args.filename, logger) - logger.info(_("session.monitorjob.percent_complete").format(0)) - error_list = [] + if not user_obj_list: + logger.info(_("importcsvsummary.line.processed").format(0)) + return + + # Apply command-line overrides to every user object before submitting. for user_obj in user_obj_list: + if args.role: + user_obj.site_role = args.role # tsc is case sensitive + if args.auth_type: + user_obj.auth_setting = args.auth_type + + # Submit as a single bulk import job. Server returns a JobItem tracking + # the async processing on its side. + try: + job = server.users.bulk_add(user_obj_list) + except Exception as e: + # includes ServerResponseError and version-gate errors on servers without bulk_add + Errors.exit_with_error(logger, exception=e) + return + + # Under --nowait the job id is the only way to track the import, so show it even when silent. + if args.nowait or not args.silent_progress: + logger.info(_("importcsvsummary.job.queued").format(job.id)) + + if args.nowait: + # Fire and forget. Server processes the job asynchronously; caller + # can query with `tabcmd get job/` or via the REST API. + return + + # Wait for the server-side job to finish. Under --silent-progress the + # framework's own debug logging is still emitted but we skip our own + # per-completion summary. + try: + job_done = server.jobs.wait_for_job(job_id=job.id, timeout=cls._timeout_seconds(args.timeout)) + except JobFailedException as je: + # JobFailedException only carries the legacy job.notes, which can be empty + # even when status_notes names the rejected row and reason. + cls._report_job_details(logger, je.job, logger.error, args.silent_progress, args.errors_file) + Errors.exit_with_error(logger, exception=je) + return + except TimeoutError as te: + # the import keeps running server-side; say so and give the id so it isn't blindly retried + logger.error(_("importcsvsummary.job.timeout").format(job.id)) + Errors.exit_with_error(logger, exception=te) + return + except Exception as e: + Errors.exit_with_error(logger, exception=e) + return + + if args.silent_progress: + # No console summary, but rejected rows must not vanish: save them to a file. + cls._report_job_details(logger, job_done, logger.info, True, args.errors_file) + return + + # Summarize per-row outcomes. `status_notes` is populated for + # UserImport jobs; each entry is a dict with keys type/value/text. + # The specific types the server emits are documented at + # https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref_jobs_tasks_and_schedules.htm#query_job + # (CountOfUsersAddedToSite, CountOfUsersSkipped, etc.). + status_notes = job_done.status_notes or [] + summary_counts = {} + for note in status_notes: + note_type = note.get("type") + note_value = note.get("value") + if note_type and note_value is not None: + summary_counts[note_type] = note_value + + added = int(summary_counts.get("CountOfUsersAddedToSite", 0) or 0) + skipped = int(summary_counts.get("CountOfUsersSkipped", 0) or 0) + processed = int(summary_counts.get("CountOfUsersProcessed", len(user_obj_list)) or 0) + + logger.info(_("importcsvsummary.line.processed").format(processed)) + logger.info(_("importcsvsummary.line.skipped").format(skipped)) + logger.info(_("importcsvsummary.users.added.count").format(added)) + + cls._report_job_details(logger, job_done, logger.info, False, args.errors_file) + + @staticmethod + def _timeout_seconds(timeout): + """--timeout arrives as a string; wait_for_job compares it with elapsed seconds.""" + try: + seconds = int(timeout) + except (TypeError, ValueError): + return None + return seconds if seconds > 0 else None + + @staticmethod + def _job_detail_lines(job): + """Per-row messages for a finished job. + + The server's statusNotes are not one entry per rejected row: when a UserImport job + has errors, it merges each error field (line/username/message/...) across every + failed row into a single statusNote per field, with values joined by ";" + (RestApiHelper.getStatusNotesErrors). That's unreadable per-row detail, so it's not + used here. The real one-line-per-row text lives in the job's legacy notes + (UserManagementService.getHumanReadableCSVImportSummary), which is what we show. + Only fall back to non-CountOf* statusNotes if notes is empty, in case some other + job type without a notes summary ever lands here. + """ + notes = [str(text) for text in getattr(job, "notes", None) or [] if text] + if notes: + return notes + status_notes = getattr(job, "status_notes", None) or [] + detail_notes = [n for n in status_notes if not (n.get("type") or "").startswith("CountOf")] + lines = [] + for note in detail_notes: + text = note.get("text") or note.get("value") or "" + lines.append(f"{note.get('type', '?')}: {text}") + return lines + + @classmethod + def _report_job_details(cls, logger, job, log_line, silent, errors_file=None): + lines = cls._job_detail_lines(job) + if not lines: + return + if silent or errors_file: + path = errors_file or f"createsiteusers-errors-{job.id}.txt" try: - if args.role: - user_obj.site_role = args.role # tsc is case sensitive - if args.auth_type: - user_obj.auth_setting = args.auth_type - number_of_users_listed += 1 - result = server.users.add(user_obj) - logger.info(_("common.output.succeeded").format(user_obj.name)) - number_of_users_added += 1 - except TSC.ServerResponseError as e: - logger.debug(e) - if Errors.is_resource_conflict(e) and args.continue_if_exists: - logger.debug(_("createsite.errors.site_name_already_exists").format(user_obj.name)) - else: - logger.debug(type(e)) - number_of_errors += 1 - logger.debug(number_of_errors) - error_list.append(e.__class__.__name__) # + ": " + e.__cause__ or "Unknown") - logger.debug(error_list) - logger.info(_("session.monitorjob.percent_complete").format(100)) - logger.info(_("importcsvsummary.line.processed").format(number_of_users_listed)) - logger.info(_("importcsvsummary.line.skipped").format(number_of_errors)) - logger.info(_("importcsvsummary.users.added.count").format(number_of_users_added)) - if number_of_errors > 0: - logger.info(_("importcsvsummary.error.details")) - logger.info(error_list) + with open(path, "w", encoding="utf-8") as f: + f.write("\n".join(lines) + "\n") + logger.error(_("importcsvsummary.error.file").format(path)) + return + except OSError: + pass # can't write the file: fall through and show the details instead + log_line(_("importcsvsummary.error.details")) + for line in lines: + log_line(f" {line}") diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py index e03b80b0..f982d60e 100644 --- a/tabcmd/execution/global_options.py +++ b/tabcmd/execution/global_options.py @@ -72,7 +72,10 @@ def set_users_file_positional(parser): def set_no_wait_option(parser): - parser.add_argument("--no-wait", action="store_true", help=_("common.options.nowait")) + # Matches tabcmd Classic's flag spelling (one word); no legacy callers to + # keep --no-wait working since this helper was never wired to a command + # before now. + parser.add_argument("--nowait", action="store_true", help=_("common.options.nowait")) return parser diff --git a/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo b/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo index bbfac751..7fc92f56 100644 Binary files a/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo and b/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo differ diff --git a/tabcmd/locales/en/shared_wg_en.properties b/tabcmd/locales/en/shared_wg_en.properties index 75328b3c..7029e857 100644 --- a/tabcmd/locales/en/shared_wg_en.properties +++ b/tabcmd/locales/en/shared_wg_en.properties @@ -9,6 +9,9 @@ content_type.datasource=Data Source dataalerts.failure.error.workbookNotFound=Workbook not found dataconnections.classes.tableau_server_site=Tableau Server Site materializeviews.errors.project_path_not_exists=Project path ''{0}'' does not exist +importcsvsummary.job.queued=User import job queued on server. Job ID: {0} +importcsvsummary.error.file=Some users were not imported. Details were saved to {0} +importcsvsummary.job.timeout=Timed out waiting for the user import job. It is still running on the server. Job ID: {0} importcsvsummary.error.details=Error details: importcsvsummary.remainingerrors=(remaining errors not shown) importcsvsummary.error.line=line {0} for user ''{1}'': ''{2}'' diff --git a/tabcmd/locales/en/tabcmd_messages_en.properties b/tabcmd/locales/en/tabcmd_messages_en.properties index a1f7272a..37d8b259 100644 --- a/tabcmd/locales/en/tabcmd_messages_en.properties +++ b/tabcmd/locales/en/tabcmd_messages_en.properties @@ -255,3 +255,4 @@ tabcmd.listing.label.owner=\tOWNER: {} tabcmd.listing.label.views=VIEWS: tabcmd.listing.summary={0} total {1} +createsiteusers.options.errors-file=Save per-row import errors to this file. Defaults to createsiteusers-errors-.txt when --silent-progress is used diff --git a/tests/assets/createsiteusers_job_with_errors.xml b/tests/assets/createsiteusers_job_with_errors.xml new file mode 100644 index 00000000..f15c8c81 --- /dev/null +++ b/tests/assets/createsiteusers_job_with_errors.xml @@ -0,0 +1,26 @@ + + + + + + + + + + processed 5 lines, skipped 0 lines, added 3 users, found 2 errors +line 3: user bob: invalid role +line 5: user alice: bad email + + diff --git a/tests/commands/test_create_site_users.py b/tests/commands/test_create_site_users.py new file mode 100644 index 00000000..d67a0b20 --- /dev/null +++ b/tests/commands/test_create_site_users.py @@ -0,0 +1,262 @@ +"""Behavior tests for createsiteusers after the switch to bulk_add. + +These are unit tests that mock the TSC layer -- they don't hit a real +server. They verify: +- default path: bulk_add called, then wait_for_job called +- --nowait: bulk_add called, wait_for_job NOT called +- --silent-progress: bulk_add + wait_for_job called, but summary log lines + are suppressed +- pre-flight validation aborts before bulk_add on malformed input under + --complete (default) +- per-row output derived from job.notes (the legacy per-row summary text) on the + completed job, not job.status_notes -- see _rejected_job and + tests/assets/createsiteusers_job_with_errors.xml for why +""" + +import argparse +import io +import os +import unittest +from unittest import mock + +import tableauserverclient as TSC + + +def _mock_open_csv(content): + """Return a mock args.filename that mimics a file with the given CSV content.""" + fp = io.StringIO(content) + fp.name = "users.csv" + return fp + + +def _base_args(**overrides): + ns = argparse.Namespace( + filename=_mock_open_csv("username,password,fullname,creator,none,yes,email\n"), + role=None, + auth_type=None, + require_all_valid=True, + continue_if_exists=False, + nowait=False, + silent_progress=False, + errors_file=None, + logging_level="INFO", + timeout=None, + username=None, + password=None, + token_name=None, + token_value=None, + server=None, + site_name="", + no_prompt=True, + no_certcheck=False, + no_proxy=False, + proxy=None, + certificate=None, + password_file=None, + token_file=None, + no_cookie=False, + query_page_size=None, + language=None, + ) + for k, v in overrides.items(): + setattr(ns, k, v) + return ns + + +class CreateSiteUsersTest(unittest.TestCase): + def _run(self, args, completed_override=None): + from tabcmd.commands.user.create_site_users import CreateSiteUsersCommand + + with ( + mock.patch("tabcmd.commands.user.create_site_users.Session") as session_cls, + mock.patch("tabcmd.commands.user.user_data.UserCommand.validate_file_for_import"), + mock.patch("tabcmd.commands.user.user_data.UserCommand.get_users_from_file") as get_users, + ): + fake_server = mock.MagicMock() + session_cls.return_value.create_session.return_value = fake_server + get_users.return_value = [ + TSC.UserItem("alice", "Creator"), + TSC.UserItem("bob", "Viewer"), + ] + + fake_job = mock.MagicMock(spec=TSC.JobItem) + fake_job.id = "abc-123" + fake_server.users.bulk_add.return_value = fake_job + + if completed_override is not None: + completed = completed_override + else: + completed = mock.MagicMock(spec=TSC.JobItem) + completed.id = "abc-123" + completed.finish_code = 0 + completed.notes = [] + completed.status_notes = [ + {"type": "CountOfUsersAddedToSite", "value": "2", "text": None}, + {"type": "CountOfUsersSkipped", "value": "0", "text": None}, + {"type": "CountOfUsersProcessed", "value": "2", "text": None}, + ] + fake_server.jobs.wait_for_job.return_value = completed + + CreateSiteUsersCommand.run_command(args) + return fake_server, fake_job, completed + + def test_default_calls_bulk_add_and_wait(self): + args = _base_args() + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_called_once_with(job_id=job.id, timeout=None) + + def test_nowait_skips_wait_for_job(self): + args = _base_args(nowait=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_not_called() + + def test_silent_progress_still_calls_wait(self): + # --silent-progress suppresses log lines, not the actual wait. + args = _base_args(silent_progress=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_called_once() + + def test_nowait_and_silent_progress_coexist(self): + args = _base_args(nowait=True, silent_progress=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_not_called() + + def test_timeout_is_passed_as_number(self): + args = _base_args(timeout="60") + server, job, completed = self._run(args) + server.jobs.wait_for_job.assert_called_once_with(job_id=job.id, timeout=60) + + def test_failed_job_logs_status_notes_and_exits(self): + from tableauserverclient.server.endpoint.exceptions import JobFailedException + + from tabcmd.commands.user.create_site_users import CreateSiteUsersCommand + + failed = mock.MagicMock(spec=TSC.JobItem) + failed.id = "abc-123" + failed.notes = ["line 3: user bob: invalid role"] + failed.status_notes = [] + with ( + mock.patch("tabcmd.commands.user.create_site_users.Session") as session_cls, + mock.patch("tabcmd.commands.user.user_data.UserCommand.validate_file_for_import"), + mock.patch( + "tabcmd.commands.user.user_data.UserCommand.get_users_from_file", + return_value=[TSC.UserItem("bob", "Viewer")], + ), + mock.patch("tabcmd.commands.user.create_site_users.log") as log_fn, + ): + server = session_cls.return_value.create_session.return_value + server.jobs.wait_for_job.side_effect = JobFailedException(failed) + with self.assertRaises(SystemExit): + CreateSiteUsersCommand.run_command(_base_args()) + logged = " ".join(str(c) for c in log_fn.return_value.error.call_args_list) + self.assertIn("line 3: user bob: invalid role", logged) + + def test_timeout_reports_job_id_and_exits(self): + from tabcmd.commands.user.create_site_users import CreateSiteUsersCommand + + with ( + mock.patch("tabcmd.commands.user.create_site_users.Session") as session_cls, + mock.patch("tabcmd.commands.user.user_data.UserCommand.validate_file_for_import"), + mock.patch( + "tabcmd.commands.user.user_data.UserCommand.get_users_from_file", + return_value=[TSC.UserItem("bob", "Viewer")], + ), + mock.patch("tabcmd.commands.user.create_site_users.log") as log_fn, + ): + server = session_cls.return_value.create_session.return_value + server.users.bulk_add.return_value.id = "job-77" + server.jobs.wait_for_job.side_effect = TimeoutError() + with self.assertRaises(SystemExit): + CreateSiteUsersCommand.run_command(_base_args(timeout="5")) + logged = " ".join(str(c) for c in log_fn.return_value.error.call_args_list) + self.assertIn("job-77", logged) + + def test_nowait_with_silent_progress_still_prints_job_id(self): + with self.assertLogs("CreateSiteUsersCommand", level="INFO") as logs: + self._run(_base_args(nowait=True, silent_progress=True)) + self.assertIn("abc-123", " ".join(logs.output)) + + def _rejected_job(self): + """A job with rejected rows, parsed from a server response shaped like the real one. + + The server doesn't emit one statusNote per rejected row -- see + tests/assets/createsiteusers_job_with_errors.xml for why. The real per-row detail + is the legacy text, which is what this job's `.notes` carries. + """ + xml_path = os.path.join(os.path.dirname(__file__), "..", "assets", "createsiteusers_job_with_errors.xml") + with open(xml_path, encoding="utf-8") as f: + xml = f.read() + return TSC.JobItem.from_response(xml, {"t": "http://tableau.com/api"})[0] + + def test_silent_progress_saves_rejected_rows_to_file(self): + import os + import tempfile + + cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmp: + os.chdir(tmp) + try: + self._run(_base_args(silent_progress=True), completed_override=self._rejected_job()) + with open("createsiteusers-errors-abc-123.txt", encoding="utf-8") as f: + content = f.read() + finally: + os.chdir(cwd) + self.assertIn("line 3: user bob: invalid role", content) + + def test_errors_file_flag_sets_custom_path(self): + import os + import tempfile + + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "custom.txt") + self._run(_base_args(silent_progress=True, errors_file=path), completed_override=self._rejected_job()) + with open(path, encoding="utf-8") as f: + content = f.read() + self.assertIn("line 3: user bob: invalid role", content) + + def test_errors_file_flag_without_silent_writes_file(self): + import os + import tempfile + + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "custom.txt") + self._run(_base_args(errors_file=path), completed_override=self._rejected_job()) + self.assertTrue(os.path.exists(path)) + + def test_silent_progress_writes_no_file_when_nothing_rejected(self): + import os + import tempfile + + cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmp: + os.chdir(tmp) + try: + self._run(_base_args(silent_progress=True)) + created = os.listdir(tmp) + finally: + os.chdir(cwd) + self.assertEqual(created, []) + + def test_not_silent_prints_rejected_rows_and_writes_no_file(self): + import os + import tempfile + + cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmp: + os.chdir(tmp) + try: + with self.assertLogs("CreateSiteUsersCommand", level="INFO") as logs: + self._run(_base_args(), completed_override=self._rejected_job()) + created = os.listdir(tmp) + finally: + os.chdir(cwd) + self.assertIn("line 3: user bob: invalid role", " ".join(logs.output)) + self.assertEqual(created, []) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/commands/test_run_commands.py b/tests/commands/test_run_commands.py index f1777d13..748401cc 100644 --- a/tests/commands/test_run_commands.py +++ b/tests/commands/test_run_commands.py @@ -428,6 +428,9 @@ def test_create_site_users(self, mock_session, mock_server): mock_args.site_name = None mock_args.role = "Viewer" mock_args.auth_type = "SAML" + mock_args.nowait = True # keep the smoke test fast; skip server-job polling + mock_args.silent_progress = False + mock_args.timeout = None create_site_users.CreateSiteUsersCommand.run_command(mock_args) mock_session.assert_called() diff --git a/tests/parsers/test_parser_create_site_users.py b/tests/parsers/test_parser_create_site_users.py index 93a51db2..b28191d1 100644 --- a/tests/parsers/test_parser_create_site_users.py +++ b/tests/parsers/test_parser_create_site_users.py @@ -48,3 +48,26 @@ def test_create_site_user_parser_auth_TabId_NotAvailable(self): mock_args = [commandname, "users.csv", "--site", "site-name", "--auth-type", "TableauId"] with self.assertRaises(SystemExit): args = self.parser_under_test.parse_args(mock_args) + + def test_create_site_user_parser_nowait(self): + # Matches Classic spelling (one word). Defaults to False -> wait. + with mock.patch("builtins.open", mock.mock_open(read_data="test")): + args = self.parser_under_test.parse_args([commandname, "users.csv"]) + assert args.nowait is False, args + + args = self.parser_under_test.parse_args([commandname, "users.csv", "--nowait"]) + assert args.nowait is True, args + + def test_create_site_user_parser_silent_progress(self): + with mock.patch("builtins.open", mock.mock_open(read_data="test")): + args = self.parser_under_test.parse_args([commandname, "users.csv"]) + assert args.silent_progress is False, args + + args = self.parser_under_test.parse_args([commandname, "users.csv", "--silent-progress"]) + assert args.silent_progress is True, args + + def test_create_site_user_parser_nowait_and_silent_coexist(self): + with mock.patch("builtins.open", mock.mock_open(read_data="test")): + args = self.parser_under_test.parse_args([commandname, "users.csv", "--nowait", "--silent-progress"]) + assert args.nowait is True, args + assert args.silent_progress is True, args