diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b23c6ab95..7ffd0f184 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -23,6 +23,6 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Install python prerequisites - run: pip install -U --user pip setuptools setuptools-scm flake8 + run: pip install -U --user pip setuptools setuptools-scm nox - name: Lint - run: python -m flake8 . + run: python -m nox --non-interactive --session validate-${{ matrix.python-version }} -k flake8 diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index aa5d4b2be..26fbb90ec 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -23,6 +23,6 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Install python prerequisites - run: pip install -U --user pip setuptools setuptools-scm mypy + run: pip install -U --user pip setuptools setuptools-scm nox - name: MyPy - run: python -m mypy cmd2 + run: python -m nox --non-interactive --session validate-${{ matrix.python-version }} -k mypy # Run nox for mypy diff --git a/CHANGELOG.md b/CHANGELOG.md index 7be7129dd..0eb53806e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.1.0 (TBD, 2021) +* Enhancements + * Converted persistent history files from pickle to compressed JSON + ## 2.0.1 (June 7, 2021) * Bug Fixes * Exclude `plugins` and `tests_isolated` directories from tarball published to PyPI for `cmd2` release diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index f8a731727..72d93f491 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -34,7 +34,6 @@ import glob import inspect import os -import pickle import pydoc import re import sys @@ -4443,15 +4442,13 @@ def _get_history(self, args: argparse.Namespace) -> 'OrderedDict[int, HistoryIte def _initialize_history(self, hist_file: str) -> None: """Initialize history using history related attributes - This function can determine whether history is saved in the prior text-based - format (one line of input is stored as one line in the file), or the new-as- - of-version 0.9.13 pickle based format. - - History created by versions <= 0.9.12 is in readline format, i.e. plain text files. - - Initializing history does not effect history files on disk, versions >= 0.9.13 always - write history in the pickle format. + :param hist_file: optional path to persistent history file. If specified, then history from + previous sessions will be included. Additionally, all history will be written + to this file when the application exits. """ + import json + import lzma + self.history = History() # with no persistent history, nothing else in this method is relevant if not hist_file: @@ -4474,36 +4471,31 @@ def _initialize_history(self, hist_file: str) -> None: self.perror(f"Error creating persistent history file directory '{hist_file_dir}': {ex}") return - # first we try and unpickle the history file - history = History() - + # Read and process history file try: with open(hist_file, 'rb') as fobj: - history = pickle.load(fobj) - except ( - AttributeError, - EOFError, - FileNotFoundError, - ImportError, - IndexError, - KeyError, - ValueError, - pickle.UnpicklingError, - ): - # If any of these errors occur when attempting to unpickle, just use an empty history + compressed_bytes = fobj.read() + history_json = lzma.decompress(compressed_bytes).decode(encoding='utf-8') + self.history = History.from_json(history_json) + except FileNotFoundError: + # Just use an empty history pass except OSError as ex: self.perror(f"Cannot read persistent history file '{hist_file}': {ex}") return + except (json.JSONDecodeError, lzma.LZMAError, KeyError, UnicodeDecodeError, ValueError) as ex: + self.perror( + f"Error processing persistent history file '{hist_file}': {ex}\n" + f"The history file will be recreated when this application exits." + ) - self.history = history self.history.start_session() self.persistent_history_file = hist_file # populate readline history if rl_type != RlType.NONE: last = None - for item in history: + for item in self.history: # Break the command into its individual lines for line in item.raw.splitlines(): # readline only adds a single entry for multiple sequential identical lines @@ -4520,14 +4512,19 @@ def _initialize_history(self, hist_file: str) -> None: atexit.register(self._persist_history) def _persist_history(self) -> None: - """Write history out to the history file""" + """Write history out to the persistent history file as compressed JSON""" + import lzma + if not self.persistent_history_file: return self.history.truncate(self._persistent_history_length) try: + history_json = self.history.to_json() + compressed_bytes = lzma.compress(history_json.encode(encoding='utf-8')) + with open(self.persistent_history_file, 'wb') as fobj: - pickle.dump(self.history, fobj) + fobj.write(compressed_bytes) except OSError as ex: self.perror(f"Cannot write persistent history file '{self.persistent_history_file}': {ex}") diff --git a/cmd2/history.py b/cmd2/history.py index c072d2e0c..df3c12558 100644 --- a/cmd2/history.py +++ b/cmd2/history.py @@ -3,12 +3,15 @@ History management classes """ +import json import re from collections import ( OrderedDict, ) from typing import ( + Any, Callable, + Dict, Iterable, List, Optional, @@ -33,6 +36,9 @@ class HistoryItem: _listformat = ' {:>4} {}' _ex_listformat = ' {:>4}x {}' + # Used in JSON dictionaries + _statement_field = 'statement' + statement: Statement = attr.ib(default=None, validator=attr.validators.instance_of(Statement)) def __str__(self) -> str: @@ -94,6 +100,22 @@ def pr(self, idx: int, script: bool = False, expanded: bool = False, verbose: bo return ret_str + def to_dict(self) -> Dict[str, Any]: + """Utility method to convert this HistoryItem into a dictionary for use in persistent JSON history files""" + return {HistoryItem._statement_field: self.statement.to_dict()} + + @staticmethod + def from_dict(source_dict: Dict[str, Any]) -> 'HistoryItem': + """ + Utility method to restore a HistoryItem from a dictionary + + :param source_dict: source data dictionary (generated using to_dict()) + :return: HistoryItem object + :raises KeyError: if source_dict is missing required elements + """ + statement_dict = source_dict[HistoryItem._statement_field] + return HistoryItem(Statement.from_dict(statement_dict)) + class History(List[HistoryItem]): """A list of :class:`~cmd2.history.HistoryItem` objects with additional methods @@ -109,6 +131,11 @@ class History(List[HistoryItem]): class to gain access to the historical record. """ + # Used in JSON dictionaries + _history_version = '1.0.0' + _history_version_field = 'history_version' + _history_items_field = 'history_items' + def __init__(self, seq: Iterable[HistoryItem] = ()) -> None: super(History, self).__init__(seq) self.session_start_index = 0 @@ -301,3 +328,36 @@ def _build_result_dictionary( if filter_func is None or filter_func(self[index]): results[index + 1] = self[index] return results + + def to_json(self) -> str: + """Utility method to convert this History into a JSON string for use in persistent history files""" + json_dict = { + History._history_version_field: History._history_version, + History._history_items_field: [hi.to_dict() for hi in self], + } + return json.dumps(json_dict, ensure_ascii=False, indent=2) + + @staticmethod + def from_json(history_json: str) -> 'History': + """ + Utility method to restore History from a JSON string + + :param history_json: history data as JSON string (generated using to_json()) + :return: History object + :raises json.JSONDecodeError: if passed invalid JSON string + :raises KeyError: if JSON is missing required elements + :raises ValueError: if history version in JSON isn't supported + """ + json_dict = json.loads(history_json) + version = json_dict[History._history_version_field] + if version != History._history_version: + raise ValueError( + f"Unsupported history file version: {version}. This application uses version {History._history_version}." + ) + + items = json_dict[History._history_items_field] + history = History() + for hi_dict in items: + history.append(HistoryItem.from_dict(hi_dict)) + + return history diff --git a/cmd2/parsing.py b/cmd2/parsing.py index 3893cb234..9069cea2b 100755 --- a/cmd2/parsing.py +++ b/cmd2/parsing.py @@ -147,6 +147,9 @@ class Statement(str): # type: ignore[override] # if output was redirected, the destination file token (quotes preserved) output_to: str = attr.ib(default='', validator=attr.validators.instance_of(str)) + # Used in JSON dictionaries + _args_field = 'args' + def __new__(cls, value: object, *pos_args: Any, **kw_args: Any) -> 'Statement': """Create a new instance of Statement. @@ -221,6 +224,31 @@ def argv(self) -> List[str]: return rtn + def to_dict(self) -> Dict[str, Any]: + """Utility method to convert this Statement into a dictionary for use in persistent JSON history files""" + return self.__dict__.copy() + + @staticmethod + def from_dict(source_dict: Dict[str, Any]) -> 'Statement': + """ + Utility method to restore a Statement from a dictionary + + :param source_dict: source data dictionary (generated using to_dict()) + :return: Statement object + :raises KeyError: if source_dict is missing required elements + """ + # value needs to be passed as a positional argument. It corresponds to the args field. + try: + value = source_dict[Statement._args_field] + except KeyError as ex: + raise KeyError(f"Statement dictionary is missing {ex} field") + + # Pass the rest at kwargs (minus args) + kwargs = source_dict.copy() + del kwargs[Statement._args_field] + + return Statement(value, **kwargs) + class StatementParser: """Parse user input as a string into discrete command components.""" diff --git a/docs/conf.py b/docs/conf.py index 08266991a..53e3c955c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -181,7 +181,7 @@ ('py:class', 'TextIO'), ('py:class', 'Union[None, Iterable, Callable]'), ('py:class', 'argparse._SubParsersAction'), - ('py:class', '_T'), + ('py:class', 'cmd2.utils._T'), ('py:class', 'StdSim'), ('py:class', 'frame'), ] diff --git a/docs/features/history.rst b/docs/features/history.rst index c84b854c3..056e02a0b 100644 --- a/docs/features/history.rst +++ b/docs/features/history.rst @@ -8,7 +8,7 @@ The ``cmd`` module from the Python standard library includes ``readline`` history. :class:`cmd2.Cmd` offers the same ``readline`` capabilities, but also maintains -it's own data structures for the history of all commands entered by the user. +its own data structures for the history of all commands entered by the user. When the class is initialized, it creates an instance of the :class:`cmd2.history.History` class (which is a subclass of ``list``) as :data:`cmd2.Cmd.history`. @@ -20,8 +20,9 @@ the parsed :class:`cmd2.Statement` is appended to :data:`cmd2.Cmd.history`. ``cmd2`` adds the option of making this history persistent via optional arguments to :meth:`cmd2.Cmd.__init__`. If you pass a filename in the ``persistent_history_file`` argument, the contents of :data:`cmd2.Cmd.history` -will be pickled into that history file. We chose to use pickle instead of plain -text so that we can save the results of parsing all the commands. +will be written as compressed JSON to that history file. We chose this format +instead of plain text to preserve the complete :class:`cmd2.Statement` object +for each command. .. note:: @@ -41,9 +42,7 @@ The :data:`cmd2.Cmd.history` attribute, the :class:`cmd2.history.History` class, and the :class:`cmd2.history.HistoryItem` class are all part of the public API for :class:`cmd2.Cmd`. You could use these classes to implement write your own ``history`` command (see below for documentation on how the -included ``history`` command works). If you don't like pickled history, you -could implement your own mechanism for saving and loading history from a plain -text file. +included ``history`` command works). For Users diff --git a/noxfile.py b/noxfile.py index 9d610f5aa..dadc53f9e 100644 --- a/noxfile.py +++ b/noxfile.py @@ -39,3 +39,10 @@ def tests(session, plugin): '--no-pty', '--append-cov', ) + + +@nox.session(python=['3.8', '3.9']) +@nox.parametrize('step', ['mypy', 'flake8']) +def validate(session, step): + session.install('invoke', './[validate]') + session.run('invoke', step) diff --git a/setup.py b/setup.py index 3289dbd13..d2d2e9550 100755 --- a/setup.py +++ b/setup.py @@ -66,19 +66,24 @@ ], # development only dependencies: install with 'pip install -e .[dev]' 'dev': [ - "pytest>=4.6", 'codecov', + 'doc8', + 'flake8', + 'invoke', + 'mypy==0.902', + 'nox', + "pytest>=4.6", 'pytest-cov', 'pytest-mock', - 'nox', - 'flake8', 'sphinx', 'sphinx-rtd-theme', 'sphinx-autobuild', - 'doc8', - 'invoke', 'twine>=1.11', ], + 'validate': [ + 'flake8', + 'mypy==0.902', + ], } PACKAGE_DATA = { diff --git a/tests/test_history.py b/tests/test_history.py index bb857334c..49959a2ea 100755 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -60,6 +60,72 @@ def hist(): return h +# Represents the hist fixture's JSON +hist_json = ( + '{\n' + ' "history_version": "1.0.0",\n' + ' "history_items": [\n' + ' {\n' + ' "statement": {\n' + ' "args": "",\n' + ' "raw": "first",\n' + ' "command": "",\n' + ' "arg_list": [],\n' + ' "multiline_command": "",\n' + ' "terminator": "",\n' + ' "suffix": "",\n' + ' "pipe_to": "",\n' + ' "output": "",\n' + ' "output_to": ""\n' + ' }\n' + ' },\n' + ' {\n' + ' "statement": {\n' + ' "args": "",\n' + ' "raw": "second",\n' + ' "command": "",\n' + ' "arg_list": [],\n' + ' "multiline_command": "",\n' + ' "terminator": "",\n' + ' "suffix": "",\n' + ' "pipe_to": "",\n' + ' "output": "",\n' + ' "output_to": ""\n' + ' }\n' + ' },\n' + ' {\n' + ' "statement": {\n' + ' "args": "",\n' + ' "raw": "third",\n' + ' "command": "",\n' + ' "arg_list": [],\n' + ' "multiline_command": "",\n' + ' "terminator": "",\n' + ' "suffix": "",\n' + ' "pipe_to": "",\n' + ' "output": "",\n' + ' "output_to": ""\n' + ' }\n' + ' },\n' + ' {\n' + ' "statement": {\n' + ' "args": "",\n' + ' "raw": "fourth",\n' + ' "command": "",\n' + ' "arg_list": [],\n' + ' "multiline_command": "",\n' + ' "terminator": "",\n' + ' "suffix": "",\n' + ' "pipe_to": "",\n' + ' "output": "",\n' + ' "output_to": ""\n' + ' }\n' + ' }\n' + ' ]\n' + '}' +) + + @pytest.fixture def persisted_hist(): from cmd2.cmd2 import ( @@ -256,6 +322,37 @@ def test_history_max_length(hist): assert hist.get(2).statement.raw == 'fourth' +def test_history_to_json(hist): + assert hist_json == hist.to_json() + + +def test_history_from_json(hist): + import json + + from cmd2.history import ( + History, + ) + + assert hist.from_json(hist_json) == hist + + # Test invalid JSON + with pytest.raises(json.JSONDecodeError): + hist.from_json("") + + # Send JSON with missing required element + with pytest.raises(KeyError): + hist.from_json("{}") + + # Create JSON with invalid history version + backed_up_ver = History._history_version + History._history_version = "BAD_VERSION" + invalid_ver_json = hist.to_json() + History._history_version = backed_up_ver + + with pytest.raises(ValueError): + hist.from_json(invalid_ver_json) + + # # test HistoryItem() # @@ -704,7 +801,7 @@ def test_exclude_from_history(base_app, monkeypatch): # @pytest.fixture(scope="session") def hist_file(): - fd, filename = tempfile.mkstemp(prefix='hist_file', suffix='.txt') + fd, filename = tempfile.mkstemp(prefix='hist_file', suffix='.dat') os.close(fd) yield filename # teardown code @@ -764,31 +861,6 @@ def test_history_file_permission_error(mocker, capsys): assert 'Cannot read' in err -def test_history_file_conversion_no_truncate_on_init(hist_file, capsys): - # make sure we don't truncate the plain text history file on init - # it shouldn't get converted to pickle format until we save history - - # first we need some plain text commands in the history file - with open(hist_file, 'w') as hfobj: - hfobj.write('help\n') - hfobj.write('alias\n') - hfobj.write('alias create s shortcuts\n') - - # Create a new cmd2 app - cmd2.Cmd(persistent_history_file=hist_file) - - # history should be initialized, but the file on disk should - # still be plain text - with open(hist_file, 'r') as hfobj: - histlist = hfobj.readlines() - - assert len(histlist) == 3 - # history.get() is overridden to be one based, not zero based - assert histlist[0] == 'help\n' - assert histlist[1] == 'alias\n' - assert histlist[2] == 'alias create s shortcuts\n' - - def test_history_populates_readline(hist_file): # - create a cmd2 with persistent history app = cmd2.Cmd(persistent_history_file=hist_file) @@ -822,7 +894,7 @@ def test_history_populates_readline(hist_file): # # test cmd2's ability to write out history on exit -# we are testing the _persist_history_on_exit() method, and +# we are testing the _persist_history() method, and # we assume that the atexit module will call this method # properly # diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 9776dace7..59b8905ba 100755 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -13,6 +13,7 @@ utils, ) from cmd2.parsing import ( + Statement, StatementParser, shlex_split, ) @@ -944,6 +945,26 @@ def test_statement_is_immutable(): statement.raw = 'baz' +def test_statement_as_dict(parser): + # Make sure to_dict() results can be restored to identical Statement + statement = parser.parse("!ls > out.txt") + assert statement == Statement.from_dict(statement.to_dict()) + + statement = parser.parse("!ls | grep text") + assert statement == Statement.from_dict(statement.to_dict()) + + statement = parser.parse("multiline arg; suffix") + assert statement == Statement.from_dict(statement.to_dict()) + + # from_dict() should raise KeyError if required field is missing + statement = parser.parse("command") + statement_dict = statement.to_dict() + del statement_dict[Statement._args_field] + + with pytest.raises(KeyError): + Statement.from_dict(statement_dict) + + def test_is_valid_command_invalid(mocker, parser): # Non-string command # noinspection PyTypeChecker