Proposal for verion 6.0.0

[r3hp1c]

BC BREAK!!
Changed using subscriber/listener to doctrine types. This does not force annotations or attributes and should allow any supported Doctrine mappings. (Probably including YAML for older Doctrine versions)
Should improve speed and memory usage.
Add tests and support for Symfony 7 and Doctrine 3. Test on PHP 8.3.
Made Halite only require dev (documentation to be updated to indicate you have to install specifically)
Namespace changes
This should close most of the open issues when done.

• Fix all checks for all versions
• Fix Encrypt/Decrypt commands for Doctrine ORM version 3 (ClassMetadata::CHANGETRACKING_NOTIFY does not exist any longer)
• Update documentation
• Update demo test cases for the DateTime, JSON and array types.
• What else was missed like composer.json require doctrine/annotations when probably no longer needed
• See other TODO messages
• Test the commands for all the supported Encrypt Types
• Squash commits

[core23]

Just two ideas what should also be part of the next major version:

• drop support for php 7 (and even 8.0) because they are no longer supported and prohibit using new features like type hints
• move YAML service configs to php service configs to improve processing time and get rid of the symfony yaml component

[r3hp1c]

@core23 thanks for the suggestions. dropping php7 support is going to make the code cleaner and possibly remove most deprecation notices. Will look into the suggestion to move to php service configuration as well.

[r3hp1c]

Firstly, all the random commits were because the lowest tests did not work as on my dev VM.

@core23 I still need to look at replacing yaml service config with php service config.

@core23 and @Zombaya I do have a task list in the description that I still need to look at, but does this proposal for version 6.0.0 close almost all open issues and are there any other suggestions and things I need to look at before I just continue with the task list?

[core23]

IMHO we can remove travis CI as we have GitHub CI for years.

The project also seems dead https://app.travis-ci.com/DoctrineEncryptBundle/DoctrineEncryptBundle

[Zombaya]

Good catch.

[r3hp1c]

Thanks for noticing that. Will get that done.

[Zombaya]

Hi @r3hp1c,

I have not done a full-in-depth review just yet, as I think we should first handle/discuss some major design choices.

Overview

Moving to doctrine/types

I'm not opposed to this, I think is certainly something we can investigate. I think we should make sure we do introduce regressions:

• Make sure we do not trigger an update every time we fetch an entity from the database, since encrypting the same string twice using halite will result in a different output.
• Make the fields of child-entities get properly decrypted (although since we are not doing weird doctrine-events anymore, I don't foresee any trouble).
• Perhaps others? Those two are two who came to mind.

I would also investigate if we would be able to introduce in the current 5.x-version of our project so that we can start showing deprecation notices there if users are using the annotation/attributes and have users migrate to the new method before having to upgrade to the next major version (similarly as how symfony does it).

Moving to seperate namespace

The new namespace is DoctrineEncryptBundle, while one would expect it to be DoctrineEncryptBundle\DoctrineEncryptBundle, given that in general you would expect name-of-organisation/name-of-project.

I'm aware this currently the same, but I foresee problems when we would ever create another project in the same organisation. For example. we would want to give that the namespace DoctrineEncryptBundle\SecondProject, which is not possible since the whole DoctrineEncryptBundle-top-namespace is claimed by this project.

I would also would like for us to try to migrate to the new namespace in a non-breaking way using the strategy as suggested by ChatGPT:

namespace Library\Old;

/**
 * @deprecated Use \Organisation\New instead.
 */
class MyClass
{
    // Your existing class implementation
}

// Provide an alias for the old namespace
class_alias(\Library\Old\MyClass::class, \Organisation\New\MyClass::class);

// You can also define a function or use an autoloader to handle the aliasing dynamically.

(I'm myself always sceptical of what ChatGPT suggest, but I have seen this kind of strategy before and I think it can work.)

Dropping support for php 7.x

I would personally prefer if we were to keep in sync with the maintained versions of symfony.

The oldest maintened version for symfony is 5.4, which supports php 7.2.5+.

I admit dropping support for 7.2 would have benefits, but I feel that this is not something a library should do.

For end-projects, I fully support and encourage dropping support for non-supported php-versions.

Move YAML service configs to php service configs

This is certainly something that can be done, but I would do this in a separate PR.

In general

I feel that this PR is trying to solve too many problems at once. I think we/you should try to split this PR up into multiple PR's, each trying to solve a single problem.

Especially if the problem is able to be solved in the current master branch without creating breaking changes, but with addition of deprecation notices if users are not doing things as the next major version will only support.

Perhaps we can enable the discussions-module for this project and start a discussion on ideas on what to develop next.

I have some ideas of my own I'd like to develop when I get to it and I feel that sharing those beforehand should help us in getting on the same page on how and when to develop things before going in head-first.

[r3hp1c]

Thanks for all your suggestions @Zombaya

I really just created this merge to ensure not to loose the code I was playing with inside a little play project I have where I test some things. O yes, and to get some feedback on the idea.

I fully agree that we need to do everything correctly and piece by piece.

Noted on the namespace changes.

Noted on PHP 7.2 and fully understand, Probably good to support all the PHP versions that the maintained Symfony versions support.

The only thing I need to figure out really is how to do the decrypt command while supporting both Annotations, Attributes and Doctrine Types.

I can ensure you we will not be doing too many updates and re-encrypts. The part I struggled to most with was getting the decrypt command to actually work. Seems that the Doctrine UOW's original value is set after the convertToPHPValue function from the type, so the comparison is never done on the encrypted value inside the UOW.

Once I get some time and thought of a way to get the Annotations, Attributes and Doctrine Types working together I will start with the real work to start the deprecations and new implementations in version 5.4.