Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

READY: Fix bugs with loading AttestationCommunity #119

Merged
merged 1 commit into from
Apr 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ipv8/attestation/wallet/community.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ def __init__(self, *args, **kwargs):
super(AttestationCommunity, self).__init__(*args, **kwargs)

self.database = AttestationsDB(working_directory, db_name)
self.attestation_request_callbacks = [lambda x, y: None, lambda x, y, z: None]
self.attestation_request_callbacks = [lambda x, y: None, lambda x, y, z, w=None: None]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments: first, I think the naming of the variables should be more clear and precise. Second, I see two callbacks here but I have no idea what they are doing exactly. Could you use a named tuple or even a dictionary instead to store these callbacks by a human-readable key?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good enhancement, but as this is a general improvement and not required for this fix I have moved this to #121 .


# Map of attestation hash -> BonehPrivateKey
self.attestation_keys = {}
for hash, _, key in self.database.get_all():
self.attestation_keys[hash] = BonehPrivateKey.unserialize(key)
self.attestation_keys[str(hash)] = BonehPrivateKey.unserialize(str(key))

self.request_cache = RequestCache()

Expand Down
26 changes: 25 additions & 1 deletion ipv8/test/attestation/wallet/test_attestation_community.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os

from ....attestation.wallet.database import AttestationsDB
from ....attestation.wallet.primitives.attestation import binary_relativity_sha512
from ....attestation.wallet.community import Attestation, AttestationCommunity, BonehPrivateKey

Expand Down Expand Up @@ -50,7 +51,7 @@ def test_request_attestation(self):
"""
Check if the request_attestation callback is correctly called.
"""
def f(peer, attribute_name, _):
def f(peer, attribute_name, _, __=None):
self.assertEqual(peer.address, self.nodes[1].endpoint.wan_address)
self.assertEqual(attribute_name, "MyAttribute")

Expand Down Expand Up @@ -101,3 +102,26 @@ def callback(rhash, values):

self.assertTrue(callback.called)
self.nodes[1].overlay.request_cache.clear()

def test_load_key(self):
"""
Check if we can load the community correctly after shut down.
"""
# Write to a temporary folder.
overlay = self.nodes[0].overlay
temp_folder = self.temporary_directory()
overlay.database = AttestationsDB(temp_folder, "test")

# Create an attestation and write it to file.
# Then close the database.
attestation = Attestation(TestCommunity.private_key.public_key(), [])
overlay.on_attestation_complete(attestation, TestCommunity.private_key, None, "test", "a"*20)
overlay.database.close(True)

# Reload the community with the same database.
self.nodes[0].overlay.__init__(self.nodes[0].my_peer, self.nodes[0].endpoint, self.nodes[0].network,
working_directory=temp_folder, db_name="test")

# The attestation should persist
db_entries = self.nodes[0].overlay.database.get_all()
self.assertEqual(1, len(db_entries))
19 changes: 15 additions & 4 deletions ipv8/test/base.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import os
import shutil
import sys
import threading
import time
Expand Down Expand Up @@ -49,10 +51,14 @@ def setUp(self):
self.__lockup_timestamp__ = time.time()

def tearDown(self):
super(TestBase, self).tearDown()
for node in self.nodes:
node.unload()
internet.clear()
try:
super(TestBase, self).tearDown()
for node in self.nodes:
node.unload()
internet.clear()
finally:
shutil.rmtree("temp", ignore_errors=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are not removing the directory created in the method temporary_directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary_directory creates a temporary directory in the temp folder (see line 139), this entire folder is rmtree'd.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I missed that one!



@classmethod
def setUpClass(cls):
Expand Down Expand Up @@ -128,3 +134,8 @@ def introduce_nodes(self):
for node in self.nodes:
node.discovery.take_step()
yield self.sleep()

def temporary_directory(self):
d = os.path.join("temp", self.__class__.__name__ + str(int(time.time())))
os.makedirs(d)
return d