-
Notifications
You must be signed in to change notification settings - Fork 10
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
Api implementation #2
Conversation
peerdid/peer_did.py
Outdated
if peer_did[9] == '0': | ||
inception_key = peer_did[11:] | ||
decoded_encnumbasis = decode_encnumbasis(inception_key, peer_did) | ||
did_doc = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move DID DOC building into a separate helper method
self.did_doc_file_extension = did_doc_file_extension or self.DEFAULT_PEERDID_FILE_EXTENSION | ||
|
||
def save(self, data: bytes): | ||
""" | ||
Saves data to a storage | ||
""" | ||
pass | ||
file_path = os.path.join(self.peer_did_storage_folder, self.peer_did_filename + self.did_doc_file_extension) | ||
with open(file_path, "bw+") as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we expect to append or override existing file?
decoded_encnumbasises = [decode_encnumbasis(key, peer_did) for key in keys_without_purpose_code] | ||
decoded_service = [decode_service(service, peer_did) for service in services] | ||
did_doc = { | ||
'id': peer_did, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to create a special entity DIDDoc (in did_doc.py
) as follows
class DIDDoc(NamedTuple):
id: str
authentication: List[PublicKeyTypeAuthentication]
key_agreement: List[PublicKeyTypeAgreement]
service: str
def to_json(self) -> str:
....
""" | ||
if peer_did is None: | ||
return False | ||
peer_did_pattern = re.compile(r'^did:peer:[01](z)([1-9a-km-zA-HJ-NP-Z]{46,47})$') | ||
peer_did_pattern = re.compile( | ||
r"^did:peer:(([0](z)([1-9a-km-zA-HJ-NP-Z]{46,47}))|(2((\.[AEVID](z)([1-9a-km-zA-HJ-NP-Z]{46,47}))+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry if I missed this discussion but is there a reason to change from the regex listed in the spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regexp in the spec appeared to be incorrect (not full) as it doesn't support numalgo2 methods for peer did creation.
:return: encoded service | ||
""" | ||
service_to_encode = re.sub(r"[\n\t\s]*", "", service) \ | ||
.replace('\'', '\"') \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit of an edge case, but could this have implications on something like
{
"serviceEndpoint": "https://myurl.com?q=some 'thing'"
}
Perhaps they should be encoded beforehand, but I think https://www.rfc-editor.org/rfc/rfc3986#section-2 -> "2.2. Reserved Characters" allows it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, i will add encoding and decoding of url special characters
No description provided.