Refactor AesEncryptor and AesDecryptor

[sofia-tekdatum]

Refactor AesEncryptor and AesDecryptor.

Move their AesCryptoContext to header file and make them inherit from EncryptorInterface and DecryptorInterface respectively.

Next step is to change the top level Encryptor to decouple it from the AesEncryptors altogether.

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[avalerio-tkd]

@sofia-tekdatum did you want to check on the tests before we continue with the refactor?

I finished this refactor before we had that conversation.

I'm back at the first PR in the chain running full tests, will report there and in subsequent PRs.

[argmarco-tkd]

Thanks for the refactor! A bit hard to read (stupid Github diffs, I know)
Left a few comments.

[argmarco-tkd]

why "2" over "DBPA" as we had it earlier? (a bit less cryptic)

[sofia-tekdatum]

I had added an explicit check in AesCryptoContext for our enum, just so we knew it was getting passed along.

That code was removed in this PR, as it's no longer needed because the refactor is getting read to do the right thing.

That message is the existing CryptoContext error message that I don't want to change. This test will go away soon when we implement External versions anyways.

[argmarco-tkd]

to help legibility, can we add a comment as to which methods are from which interface?
Same for decryptor

[sofia-tekdatum]

Done, I added a start/end marker for the Encryptor/Decryptor interface methods.

[argmarco-tkd]

Left a few comments - not blockers to merge.

[sofia-tekdatum]

@sofia-tekdatum did you want to check on the tests before we continue with the refactor?

I finished this refactor before we had that conversation.

I'm back at the first PR in the chain running full tests, will report there and in subsequent PRs.

@avalerio-tkd after the test investigation, ran the following:

From cpp/build:
./release/parquet-encryption-test
./release/parquet-encryption-key-management-test
./release/arrow-dataset-file-parquet-encryption-test

From python:
python -m pytest pyarrow/tests/parquet/test_encryption.py
python -m pytest pyarrow/tests/parquet/test_external_encryption.py
python -m pytest pyarrow/tests/test_dataset_encryption.py

All passed.

Can you PTAL?

[avalerio-tkd]

Already had a look previously and LGTM.

@sofia-tekdatum thanks 10x for the diligence with the tests. That's reassuring.