jolocom / jolocom-lib

Library for interacting with the identity solution provided by Jolocom.
MIT License
24 stars 18 forks source link

Moved validation logic to validateJWT. Slight refactor #359

Closed Exulansis closed 5 years ago

Exulansis commented 5 years ago

Closes #358.

Exulansis commented 5 years ago

I think the logic enclosed in handleValidationState is nice to have contained in something reusable. It can possibly go where all the validation logic will go (or just kept around now till moved).

I think the handleValidationState is not too reusable. It is defined as

(condition, key) => !condition 
    ? throw new Error(errorMap[key])
    : undefined

So it takes a bool, and throws an error in case the bool is false, with an error message from a local map (which is currently tailored for token validation).

Exulansis commented 5 years ago

HandleValidationState currently abstracts an if statement + encapsulates a map of error messages.

mnzaki commented 5 years ago

HandleValidationState currently abstracts an if statement + encapsulates a map of error messages.

I think we do need to encapsulate the validation errors though, because external code will want to find out what kind of error it is (so probably some check with error codes from an enum). Although the current implementation of handleValidationState wasn't really useful to that end, especially because the error codes are not exported and are flattened out to the error messages as well... so yeah, off with its head

Exulansis commented 5 years ago

HandleValidationState currently abstracts an if statement + encapsulates a map of error messages.

I think we do need to encapsulate the validation errors though, because external code will want to find out what kind of error it is (so probably some check with error codes from an enum). Although the current implementation of handleValidationState wasn't really useful to that end, especially because the error codes are not exported and are flattened out to the error messages as well... so yeah, off with its head

+1 I think an issue can be created to have some better overarching way to do this. Error code + reusing messages where suitable is a good idea.