Skip to content

Commit

Permalink
Merge branch 'release-7.7' into devel
Browse files Browse the repository at this point in the history
  • Loading branch information
drew2a committed Jan 11, 2021
2 parents 88e9490 + 47dbcfd commit b29e4a6
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/run_tribler.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async def start_tribler():

state_dir = get_versioned_state_directory(root_state_dir)

config = TriblerConfig(state_dir, config_file=state_dir / CONFIG_FILENAME)
config = TriblerConfig(state_dir, config_file=state_dir / CONFIG_FILENAME, reset_config_on_error=True)

if not config.get_core_error_reporting_requires_user_consent():
SentryReporter.global_strategy = SentryStrategy.SEND_ALLOWED
Expand Down
1 change: 1 addition & 0 deletions src/tribler-common/tribler_common/simpledefs.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class NTFY(Enum):
EVENTS_START = "events_start"
TRIBLER_EXCEPTION = "tribler_exception"
POPULARITY_COMMUNITY_ADD_UNKNOWN_TORRENT = "PopularityCommunity:added_unknown_torrent"
REPORT_CONFIG_ERROR = "report_config_error"


class CHANNEL_STATE(Enum):
Expand Down
28 changes: 28 additions & 0 deletions src/tribler-core/tribler_core/config/test_tribler_config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import shutil
from pathlib import Path

from configobj import ParseError

import pytest

from tribler_common.simpledefs import MAX_LIBTORRENT_RATE_LIMIT

from tribler_core.config.tribler_config import CONFIG_FILENAME, TriblerConfig
from tribler_core.tests.tools.common import TESTS_DATA_DIR
from tribler_core.utilities.osutils import get_home_dir

CONFIG_PATH = TESTS_DATA_DIR / "config_files"


def test_init_without_config(tribler_config):
"""
Expand All @@ -13,6 +21,26 @@ def test_init_without_config(tribler_config):
tribler_config.validate()


def test_invalid_config_recovers(tmpdir):
default_config_file = tmpdir / 'triblerd.conf'
shutil.copy2(CONFIG_PATH / 'corrupt-triblerd.conf', default_config_file)

# By default, recover_error set to False when loading the config file so
# if the config file is corrupted, it should raise a ParseError.
with pytest.raises(ParseError):
_ = TriblerConfig(tmpdir, config_file=default_config_file)

# If recover_error is set to True, the config should successfully load using
# the default config in case of corrupted config file and the error is saved.
tribler_conf = TriblerConfig(tmpdir, config_file=default_config_file, reset_config_on_error=True)
assert "configobj.ParseError: Invalid line" in tribler_conf.config_error
assert tribler_conf.get_state_dir() is not None

# Since the config should be saved on previous recovery, subsequent instantiation of TriblerConfig
# should work without the reset flag.
tribler_conf2 = TriblerConfig(tmpdir, config_file=default_config_file, reset_config_on_error=False)
assert tribler_conf2

def test_write_load(tribler_config):
"""
When writing and reading a config the options should remain the same.
Expand Down
37 changes: 32 additions & 5 deletions src/tribler-core/tribler_core/config/tribler_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
"""
import logging
import os
import traceback
from typing import Optional

from configobj import ConfigObj
from configobj import ConfigObj, ParseError

from validate import Validator

Expand All @@ -30,26 +32,51 @@ class TriblerConfig:
their allowed values and default value in `config.spec`.
"""

def __init__(self, state_dir, config_file=None):
def __init__(self, state_dir, config_file=None, reset_config_on_error=False):
"""
Create a new TriblerConfig instance.
:param config_file: path to existing config file
:param reset_config_on_error: Flag indicating whether to recover from corrupt config using default config.
:raises an InvalidConfigException if ConfigObj is invalid
"""
self._logger = logging.getLogger(self.__class__.__name__)
self._state_dir = Path(state_dir)
self.config = ConfigObj(infile=(str(config_file) if config_file else None),
configspec=str(CONFIG_SPEC_PATH), default_encoding='utf-8')
self.validate()
self.selected_ports = {}

self.config = None
self.config_error = None
self.load_config_file(config_file, reset_config_on_error)

# Set the default destination dir. The value should be in the config dict
# because of the REST endpoint sending the whole dict to the GUI.
# TODO: do not write the value into the config file if the user did not change it
if self.config['download_defaults']['saveas'] is None:
self.config['download_defaults']['saveas'] = str(self.abspath(get_default_dest_dir()))

def load_config_file(self, config_file: Path, recover_error: bool):
"""
Loads ConfigObj from the config file. If the file is corrupted, tries to create
ConfigObj using the spec file if `recover_error` is set.
"""
config_file_path = str(config_file) if config_file else None
try:
self.config = self._load_config_file(config_file_path)
except ParseError:
if not config_file or not recover_error:
raise

self.config_error = traceback.format_exc()
self.config = self._load_config_file(None)
# Since this is a new config, save it immediately
self.write()

self.validate()

def _load_config_file(self, config_file_path: Optional[str]) -> ConfigObj:
return ConfigObj(infile=config_file_path, configspec=str(CONFIG_SPEC_PATH), default_encoding='utf-8')

def abspath(self, path):
return path_util.abspath(path, optional_prefix=self.get_state_dir())

Expand Down
2 changes: 2 additions & 0 deletions src/tribler-core/tribler_core/restapi/events_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ def passthrough(x):
NTFY.TRIBLER_STARTED: lambda public_key: {"version": version_id, "public_key": hexlify(public_key)},
# Tribler is low on disk space for storing torrents
NTFY.LOW_SPACE: passthrough,
# Report config error on startup
NTFY.REPORT_CONFIG_ERROR: passthrough,
}
# pylint: enable=line-too-long

Expand Down
4 changes: 4 additions & 0 deletions src/tribler-core/tribler_core/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,10 @@ async def start(self):

self.notifier.notify(NTFY.TRIBLER_STARTED, self.trustchain_keypair.key.pk)

# If there is a config error, report to the user via GUI notifier
if self.config.config_error:
self.notifier.notify(NTFY.REPORT_CONFIG_ERROR, self.config.config_error)

async def shutdown(self):
"""
Checkpoints the session and closes it, stopping the download engine.
Expand Down
37 changes: 36 additions & 1 deletion src/tribler-core/tribler_core/tests/test_session.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import asyncio
import shutil
from asyncio import get_event_loop, sleep
from unittest.mock import Mock

Expand All @@ -7,7 +9,11 @@

import pytest

from tribler_core.session import IGNORED_ERRORS
from tribler_common.simpledefs import NTFY

from tribler_core.config.test_tribler_config import CONFIG_PATH
from tribler_core.config.tribler_config import TriblerConfig
from tribler_core.session import IGNORED_ERRORS, Session
from tribler_core.tests.tools.base_test import MockObject

# Pylint does not agree with the way pytest handles fixtures.
Expand Down Expand Up @@ -149,3 +155,32 @@ async def test_load_ipv8_overlays_testnet(mocked_endpoints, enable_ipv8, session
assert "PopularityCommunity" in loaded_launchers
assert "GigaChannelCommunity" not in loaded_launchers
assert "GigaChannelTestnetCommunity" in loaded_launchers


@pytest.mark.asyncio
async def test_config_error_notification(tmpdir):
"""
Test if config error is notified on session startup.
"""

# 1. Setup corrupted config
shutil.copy2(CONFIG_PATH / 'corrupt-triblerd.conf', tmpdir / 'triblerd.conf')
tribler_config = TriblerConfig(tmpdir, config_file=tmpdir / 'triblerd.conf', reset_config_on_error=True)

# 2. Initialize session with corrupted config, disable upgrader for simplicity.
# By mocking the notifier, we can check if the notify was called with correct parameters
session = Session(tribler_config)
session.upgrader_enabled = False
session.notifier = Mock()

# 3. Start the session which should internally call the notifier.
await session.start()
# Notifier uses asyncio loop internally, so we must wait at least a single loop cycle
await asyncio.sleep(0)

# 4. There could be multiple notify calls but we are only interested in the config error
# so using 'assert_any_call' here to check if the notify was called.
session.notifier.notify.assert_any_call(NTFY.REPORT_CONFIG_ERROR, tribler_config.config_error)

# 5. Always shutdown the session at the end.
await session.shutdown()
Binary file not shown.
2 changes: 2 additions & 0 deletions src/tribler-gui/tribler_gui/event_request_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class EventRequestManager(QNetworkAccessManager):
low_storage_signal = pyqtSignal(object)
tribler_shutdown_signal = pyqtSignal(str)
change_loading_text = pyqtSignal(str)
config_error_signal = pyqtSignal(str)

def __init__(self, api_port, api_key):
QNetworkAccessManager.__init__(self)
Expand All @@ -61,6 +62,7 @@ def __init__(self, api_port, api_key):
NTFY.TRIBLER_SHUTDOWN_STATE.value: self.tribler_shutdown_signal.emit,
NTFY.EVENTS_START.value: self.events_start_received,
NTFY.TRIBLER_STARTED.value: self.tribler_started_event,
NTFY.REPORT_CONFIG_ERROR.value: self.config_error_signal.emit,
}

def events_start_received(self, event_dict):
Expand Down
21 changes: 20 additions & 1 deletion src/tribler-gui/tribler_gui/tests/test_util.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from urllib.parse import unquote_plus

from tribler_gui.utilities import compose_magnetlink, quote_plus_unicode, unicode_quoter
from tribler_gui.utilities import compose_magnetlink, dict_item_is_any_of, quote_plus_unicode, unicode_quoter


def test_quoter_char():
Expand Down Expand Up @@ -105,3 +105,22 @@ def test_compose_magnetlink():
assert composed_link1 == expected_link1
assert composed_link2 == expected_link2
assert composed_link3 == expected_link3


def test_is_dict_has():
assert not dict_item_is_any_of(None, None, None)
assert not dict_item_is_any_of({}, None, None)

d = {
'k': 'v',
'k1': 'v1'
}

assert not dict_item_is_any_of(d, 'missed_key', None)
assert not dict_item_is_any_of(d, 'missed_key', ['any_value'])
assert not dict_item_is_any_of(d, 'k', ['missed_value'])
assert not dict_item_is_any_of(d, 'k', ['missed_value', 'missed_value1'])

assert dict_item_is_any_of(d, 'k', ['v'])
assert dict_item_is_any_of(d, 'k', ['v', 'a'])
assert dict_item_is_any_of(d, 'k', ['a', 'v'])
6 changes: 6 additions & 0 deletions src/tribler-gui/tribler_gui/tribler_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def on_exception(self, *exc_info):

def __init__(self, core_args=None, core_env=None, api_port=None, api_key=None):
QMainWindow.__init__(self)
self._logger = logging.getLogger(self.__class__.__name__)

QCoreApplication.setOrganizationDomain("nl")
QCoreApplication.setOrganizationName("TUDelft")
Expand Down Expand Up @@ -323,6 +324,7 @@ def on_state_update(new_state):
connect(self.core_manager.events_manager.tribler_started, self.on_tribler_started)
connect(self.core_manager.events_manager.low_storage_signal, self.on_low_storage)
connect(self.core_manager.events_manager.tribler_shutdown_signal, self.on_tribler_shutdown_state_update)
connect(self.core_manager.events_manager.config_error_signal, self.on_config_error_signal)

# Install signal handler for ctrl+c events
def sigint_handler(*_):
Expand Down Expand Up @@ -1178,6 +1180,10 @@ def on_skip_conversion_dialog(self, action):
def on_tribler_shutdown_state_update(self, state):
self.loading_text_label.setText(state)

def on_config_error_signal(self, stacktrace):
self._logger.error(f"Config error: {stacktrace}")
user_message = 'Tribler recovered from a corrupted config. Please check your settings and update if necessary.'
ConfirmationDialog.show_error(self, "Tribler config error", user_message)

def _qurl_to_path(qurl):
parsed = urlparse(qurl.toString())
Expand Down
6 changes: 6 additions & 0 deletions src/tribler-gui/tribler_gui/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,3 +422,9 @@ def disconnect(signal: pyqtSignal, callback: Callable):

class CreationTraceback(Exception):
pass


def dict_item_is_any_of(d, key, values):
if not d or not key or not values:
return False
return key in d and d[key] in values
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from tribler_gui.defs import HEALTH_CHECKING, HEALTH_UNCHECKED
from tribler_gui.tribler_action_menu import TriblerActionMenu
from tribler_gui.tribler_request_manager import TriblerNetworkRequest
from tribler_gui.utilities import connect, get_health
from tribler_gui.utilities import connect, dict_item_is_any_of, get_health

HEALTHCHECK_DELAY_MS = 500

Expand Down Expand Up @@ -117,7 +117,7 @@ def _on_selection_changed(self, selected, deselected):
return

data_item = selected_indices[-1].model().data_items[selected_indices[-1].row()]
if 'type' in data_item and data_item['type'] != REGULAR_TORRENT:
if not dict_item_is_any_of(data_item, 'type', [REGULAR_TORRENT]):
return

# Trigger health check if necessary
Expand All @@ -134,6 +134,9 @@ def _on_selection_changed(self, selected, deselected):
class HealthCheckerMixin:
def check_torrent_health(self, data_item, forced=False):
# TODO: stop triggering multiple checks over a single infohash by e.g. selection and click signals
if not dict_item_is_any_of(data_item, 'type', [REGULAR_TORRENT]):
return

infohash = data_item['infohash']

if 'health' not in self.model.column_position:
Expand Down Expand Up @@ -289,7 +292,7 @@ def add_menu_item(self, menu, name, item_index, callback):
def selection_can_be_added_to_channel(self):
for row in self.table_view.selectionModel().selectedRows():
data_item = row.model().data_items[row.row()]
if 'type' in data_item and data_item['type'] in (REGULAR_TORRENT, CHANNEL_TORRENT, COLLECTION_NODE):
if dict_item_is_any_of(data_item, 'type', [REGULAR_TORRENT, CHANNEL_TORRENT, COLLECTION_NODE]):
return True
return False

Expand Down

0 comments on commit b29e4a6

Please sign in to comment.