From 3bfd9f487b67d74b8990a8fd5e0141510b94db54 Mon Sep 17 00:00:00 2001 From: Micah Jerome Ellison <4383304+micahellison@users.noreply.github.com> Date: Sat, 24 Aug 2019 13:50:10 -0700 Subject: [PATCH 1/5] [GH-632] confirming that each journal can be parsed during upgrade, and aborting upgrade if not --- features/encryption.feature | 11 ----------- features/regression.feature | 11 ----------- features/upgrade.feature | 23 +++++++++++++++++++++++ jrnl/Journal.py | 13 ++++++++++++- jrnl/upgrade.py | 23 +++++++++++++++++------ 5 files changed, 52 insertions(+), 29 deletions(-) create mode 100644 features/upgrade.feature diff --git a/features/encryption.feature b/features/encryption.feature index ebb3cc02..82d971eb 100644 --- a/features/encryption.feature +++ b/features/encryption.feature @@ -29,14 +29,3 @@ When we run "jrnl simple -n 1" Then we should not see the message "Password" and the output should contain "2013-06-10 15:40 Life is good" - - Scenario: Upgrading a journal encrypted with jrnl 1.x - Given we use the config "encrypted_old.json" - When we run "jrnl -n 1" and enter - """ - Y - bad doggie no biscuit - bad doggie no biscuit - """ - Then we should see the message "Password" - and the output should contain "2013-06-10 15:40 Life is good" diff --git a/features/regression.feature b/features/regression.feature index 21078a1c..3e644b19 100644 --- a/features/regression.feature +++ b/features/regression.feature @@ -43,17 +43,6 @@ Feature: Zapped bugs should stay dead. | Hope to get a lot of traffic. """ - Scenario: Upgrade and parse journals with square brackets - Given we use the config "upgrade_from_195.json" - When we run "jrnl -9" and enter "Y" - Then the output should contain - """ - 2010-06-10 15:00 A life without chocolate is like a bad analogy. - - 2013-06-10 15:40 He said "[this] is the best time to be alive". - """ - Then the journal should have 2 entries - Scenario: Integers in square brackets should not be read as dates Given we use the config "brackets.yaml" When we run "jrnl -1" diff --git a/features/upgrade.feature b/features/upgrade.feature new file mode 100644 index 00000000..bce026b8 --- /dev/null +++ b/features/upgrade.feature @@ -0,0 +1,23 @@ +Feature: Upgrading Journals from 1.x.x to 2.x.x + + Scenario: Upgrade and parse journals with square brackets + Given we use the config "upgrade_from_195.json" + When we run "jrnl -9" and enter "Y" + Then the output should contain + """ + 2010-06-10 15:00 A life without chocolate is like a bad analogy. + + 2013-06-10 15:40 He said "[this] is the best time to be alive". + """ + Then the journal should have 2 entries + + Scenario: Upgrading a journal encrypted with jrnl 1.x + Given we use the config "encrypted_old.json" + When we run "jrnl -n 1" and enter + """ + Y + bad doggie no biscuit + bad doggie no biscuit + """ + Then we should see the message "Password" + and the output should contain "2013-06-10 15:40 Life is good" diff --git a/jrnl/Journal.py b/jrnl/Journal.py index cf336bdc..7feb49a6 100644 --- a/jrnl/Journal.py +++ b/jrnl/Journal.py @@ -84,9 +84,20 @@ class Journal(object): def write(self, filename=None): """Dumps the journal into the config file, overwriting it""" filename = filename or self.config['journal'] - text = "\n".join([e.__unicode__() for e in self.entries]) + text = self._to_text() self._store(filename, text) + def validate_parsing(self): + """Confirms that the jrnl is still parsed correctly after being dumped to text.""" + new_entries = self._parse(self._to_text()) + for i, entry in enumerate(self.entries): + if entry != new_entries[i]: + return False + return True + + def _to_text(self): + return "\n".join([e.__unicode__() for e in self.entries]) + def _load(self, filename): raise NotImplementedError diff --git a/jrnl/upgrade.py b/jrnl/upgrade.py index 3784c9f6..1f4f999a 100644 --- a/jrnl/upgrade.py +++ b/jrnl/upgrade.py @@ -44,6 +44,7 @@ older versions of jrnl anymore. encrypted_journals = {} plain_journals = {} other_journals = {} + all_journals = [] for journal_name, journal_conf in config['journals'].items(): if isinstance(journal_conf, dict): @@ -85,17 +86,27 @@ older versions of jrnl anymore. util.prompt("\nUpgrading encrypted '{}' journal stored in {}...".format(journal_name, path)) backup(path, binary=True) old_journal = Journal.open_journal(journal_name, util.scope_config(config, journal_name), legacy=True) - new_journal = EncryptedJournal.from_journal(old_journal) - new_journal.write() - util.prompt(" Done.") + all_journals.append(EncryptedJournal.from_journal(old_journal)) for journal_name, path in plain_journals.items(): util.prompt("\nUpgrading plain text '{}' journal stored in {}...".format(journal_name, path)) backup(path) old_journal = Journal.open_journal(journal_name, util.scope_config(config, journal_name), legacy=True) - new_journal = Journal.PlainJournal.from_journal(old_journal) - new_journal.write() - util.prompt(" Done.") + all_journals.append(Journal.PlainJournal.from_journal(old_journal)) + + # loop through lists to validate + failed_journals = [j for j in all_journals if not j.validate_parsing()] + if len(failed_journals) > 0: + util.prompt("\nThe following journal{} failed to upgrade:\n{}".format( + 's' if len(failed_journals) > 1 else '', "\n".join(j.name for j in failed_journals)) + ) + + util.prompt("Aborting upgrade.") + return + + # write all journals - or - don't + for j in all_journals: + j.write() util.prompt("\nUpgrading config...") backup(config_path) From a4e881942a17cde05cd16b2bc34e92998ed9dea7 Mon Sep 17 00:00:00 2001 From: Micah Jerome Ellison <4383304+micahellison@users.noreply.github.com> Date: Sat, 24 Aug 2019 15:01:23 -0700 Subject: [PATCH 2/5] [GH-632] raising exception in upgrade.py on fail Handling it in install.py to prevent config from being overwritten when upgrade fails --- jrnl/install.py | 10 +++++++++- jrnl/upgrade.py | 8 +++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/jrnl/install.py b/jrnl/install.py index 7cc8f880..68cfb6ea 100644 --- a/jrnl/install.py +++ b/jrnl/install.py @@ -14,6 +14,7 @@ from .Journal import PlainJournal from .EncryptedJournal import EncryptedJournal import yaml import logging +import sys DEFAULT_CONFIG_NAME = 'jrnl.yaml' DEFAULT_JOURNAL_NAME = 'journal.txt' @@ -85,8 +86,15 @@ def load_or_install_jrnl(): if os.path.exists(config_path): log.debug('Reading configuration from file %s', config_path) config = util.load_config(config_path) - upgrade.upgrade_jrnl_if_necessary(config_path) + + try: + upgrade.upgrade_jrnl_if_necessary(config_path) + except upgrade.UpgradeValidationException: + util.prompt("Aborting upgrade. Exiting.") + sys.exit(1) + upgrade_config(config) + return config else: log.debug('Configuration file not found, installing jrnl...') diff --git a/jrnl/upgrade.py b/jrnl/upgrade.py index 1f4f999a..b3d39984 100644 --- a/jrnl/upgrade.py +++ b/jrnl/upgrade.py @@ -96,12 +96,14 @@ older versions of jrnl anymore. # loop through lists to validate failed_journals = [j for j in all_journals if not j.validate_parsing()] + if len(failed_journals) > 0: util.prompt("\nThe following journal{} failed to upgrade:\n{}".format( 's' if len(failed_journals) > 1 else '', "\n".join(j.name for j in failed_journals)) ) - util.prompt("Aborting upgrade.") + raise UpgradeValidationException + return # write all journals - or - don't @@ -112,3 +114,7 @@ older versions of jrnl anymore. backup(config_path) util.prompt("\nWe're all done here and you can start enjoying jrnl 2.".format(config_path)) + +class UpgradeValidationException(Exception): + """Raised when the contents of an upgraded journal do not match the old journal""" + pass From 4d0640e613761a2007533db2669e03fe583c829a Mon Sep 17 00:00:00 2001 From: Micah Jerome Ellison <4383304+micahellison@users.noreply.github.com> Date: Sat, 24 Aug 2019 15:14:15 -0700 Subject: [PATCH 3/5] [GH-632] removing unnecessary whitespace --- jrnl/install.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jrnl/install.py b/jrnl/install.py index 68cfb6ea..9b6965fa 100644 --- a/jrnl/install.py +++ b/jrnl/install.py @@ -86,7 +86,7 @@ def load_or_install_jrnl(): if os.path.exists(config_path): log.debug('Reading configuration from file %s', config_path) config = util.load_config(config_path) - + try: upgrade.upgrade_jrnl_if_necessary(config_path) except upgrade.UpgradeValidationException: @@ -94,7 +94,7 @@ def load_or_install_jrnl(): sys.exit(1) upgrade_config(config) - + return config else: log.debug('Configuration file not found, installing jrnl...') From 244165664b125da0b3c2897e6d1ddc3d6f1c6ce6 Mon Sep 17 00:00:00 2001 From: Micah Jerome Ellison <4383304+micahellison@users.noreply.github.com> Date: Sat, 24 Aug 2019 15:25:05 -0700 Subject: [PATCH 4/5] [GH-632] removing unreachable return statement --- jrnl/upgrade.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/jrnl/upgrade.py b/jrnl/upgrade.py index b3d39984..4e315f82 100644 --- a/jrnl/upgrade.py +++ b/jrnl/upgrade.py @@ -104,8 +104,6 @@ older versions of jrnl anymore. raise UpgradeValidationException - return - # write all journals - or - don't for j in all_journals: j.write() From be35904912efe6284fff70461830fe0ab65ca60b Mon Sep 17 00:00:00 2001 From: Micah Jerome Ellison <4383304+micahellison@users.noreply.github.com> Date: Sat, 24 Aug 2019 15:47:13 -0700 Subject: [PATCH 5/5] [GH-632] adding call to action to report issue when upgrade fails --- jrnl/install.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jrnl/install.py b/jrnl/install.py index 9b6965fa..417497aa 100644 --- a/jrnl/install.py +++ b/jrnl/install.py @@ -90,7 +90,10 @@ def load_or_install_jrnl(): try: upgrade.upgrade_jrnl_if_necessary(config_path) except upgrade.UpgradeValidationException: - util.prompt("Aborting upgrade. Exiting.") + util.prompt("Aborting upgrade.") + util.prompt("Please tell us about this problem at the following URL:") + util.prompt("https://github.com/jrnl-org/jrnl/issues/new?title=UpgradeValidationException") + util.prompt("Exiting.") sys.exit(1) upgrade_config(config)