Empty or corrupt files detection and error + code cleanup

[buchdag]

This a first take at what's been discussed on #23

730831b just reorder plugins and unit test classes to something a bit more readable and logical to me. There is zero code change on this commit.

3f11801 change the AccountKey() KeyFile() CertFile() ChainFile() FullChainFile() and FullFile() IOPlugins classes to be more similar. FullChainFile() is no longer a subclass of ChainFile(). The 3 "chain" plugins use a list built from the iterator created by split_pems() to allow testing its length (required to catch issues with those files later).

e078cee split_pems() is only used by subclasses of OpenSSLIOPlugin() so it make more sense to me to make it a method of this class rather than a function. The test performed by the function doctest has been moved to the unit test for the IOPlugins. I'm not 100% happy with this has it means it will be tested again for each plugin but that's the solution with the least amount of additional code (I can detail the other possibility I thought of).

27b822d same as above with load_pem_jwk() and dump_pem_jwk() functions being moved to methods of the JWKIOPlugin() class, for the same reasons.

158fef0 catch openssl errors on the OpenSSLIOPlugin() methods. This is enough for the "single files" plugins as empty or invalid content will result in openssl throwing an error.

3c5b4aa catch empty file for the "chain" plugins or files that didn't contain enough discrete pem encoded data + tests for this and the openssl error

001d80a fix for a new check added in Pylint 1.8.0 that simp_le was failing.

[zenhack]

This looks mostly sensible to me. I do wish git had more useful diffs for when a big chunk of code has just been moved without modifications.

re: the split_pems test, you could just add it to its own test case class, without any of the mixins. What was your other idea?

[zenhack]

Grammar issue: you want either a period or a semicolon here, not a comma.

[zenhack]

Could just do list(self.split_pems(content))

[zenhack]

Grammar:

1. s/on/in/
2. Use a semicolon, not a comma.

Same thing the other places this message shows up.

[buchdag]

re: the split_pems test, you could just add it to its own test case class, without any of the mixins. What was your other idea?

Yes, this is the other code I tested:

@OpenSSLIOPlugin.register(path='test.pem')
class OpenSSLIOTest(OpenSSLIOPlugin, unittest.TestCase):
    """OpenSSL IO plugin tests."""
    # this is a test suite | pylint: disable=missing-docstring

    PEM = b'\n-----BEGIN FOO BAR-----\nfoo\nbar\n-----END FOO BAR-----'

    def persisted(self):
        pass

    def load(self):
        pass

    def save(self, data):
        pass

    def test_split_pems(self):
        result = len(list(self.split_pems(self.PEM * 3)))
        self.assertEqual(result, 3)
        result = list(self.split_pems(b''))
        self.assertEqual(result, [])

That's longer but it makes more sense to me than performing this exact test five more times. Do you prefer that too ?

[zenhack]

If you make split pems a stand alone function, you can inherit just from unittest.TestCase, so you don't need the stub methods. I think this makes a bit more sense anyway, since it doesn't need self for anything.

[buchdag]

I do agree with the fact that making split_pems() a method of OpenSSLIOPlugin() is not required per se but I still think doing it enhance the readability and structure, just like making jwk operations class methods rather than functions.

[zenhack]

Might be cleaner to get the structure by splitting things into files, but that doesn't need to happen now.

You could also make it a static method and call it as OpenSSLIOPlugin.spit_pems(), rather than self.

[buchdag]

I did the requested changes and made split_pems() a static method, let me know if this suits you or if you prefer to revert split_pems() to a function.

[buchdag]

note: there is nothing similar yet for the account_key.json file.

[zenhack]

Okay, looks good to me, merging.