Skip to content
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

Re-add OAuth1 dependency #10196

Closed
wants to merge 1 commit into from

Conversation

dennisameling
Copy link
Member

@dennisameling dennisameling commented Jun 28, 2021

As can be seen in https://github.com/mautic/mautic/runs/2935394374?check_suite_focus=true, the OAuth1 removal PR breaks the update from an earlier Mautic version to M4 RC.

My assumption is that the code, during the update, still expects the willdurand/oauth-server-bundle dependency to be present (Symfony's container/cache maybe?), even though the consuming OAuth1 code has been removed in said PR.

My suggested approach is to re-add the dependency, while this won't have any end-user impact as the OAuth1 functionality will still be absent in Mautic itself. Then, in e.g. 4.0.1 or 4.1.0, we can safely remove the dependency and make sure that that version requires at least Mautic 4.0.0 as the minimum installed version, since 4.0.0 will then remove the consuming OAuth1 code.

Easiest way to test is by checking out this PR, generating a release package with php build/package_release.php -b oauth1-quickfix, then with a Mautic 3 instance, apply the update package with bin/console mautic:update:apply --update-package=4.0.0-beta-update.zip. Or, even simpler: merge this PR "as-is" and kick off the release process 😉

Fixes #10195

@dennisameling dennisameling added dependencies Pull requests that update a dependency file CI Anything related to CI (Continuous Integration) chore Tasks that relate to maintaining this Github repository labels Jun 28, 2021
@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Jun 28, 2021
@alanhartless
Copy link
Contributor

@dennisameling my hunch is there's something more at play here. We hit similar issues with moving from 2 to 3 because of the dependencies changing significantly and changes in how Symfony built the container (multiple files vs single) resulting in a requirement to leverage a two step process (running mautic:update:apply) twice. By the time that the command is ran with --finalize, the new code should be in place which means app/bundles/ApiBundle/Entity/oAuth1/Consumer.php shouldn't even exist.

PHP Fatal error:  Uncaught Error: Interface 'Bazinga\OAuthServerBundle\Model\ConsumerInterface' not found in /home/runner/work/mautic/mautic/mautic-testing/app/bundles/ApiBundle/Entity/oAuth1/Consumer.php:25

Is there a way to verify if files that were supposed to be deleted were actually deleted? https://github.com/acquia/mc-cs/blob/edc9159d81b4272c0b601ff55d9ca7a8c009f403/app/bundles/CoreBundle/Command/ApplyUpdatesCommand.php#L195-L219

@dennisameling
Copy link
Member Author

Yeah it's weird that the file is still there - maybe because it was still in use by the Symfony kernel? Or the update script maybe won't delete files that are no longer present in the update package? Not sure.

In case the file is still in use by the kernel - on mobile now so can't validate, but we might be able to somehow shut down the Symfony kernel and only start moving/deleting files after that

https://hotexamples.com/examples/symfony.component.httpkernel/Kernel/shutdown/php-kernel-shutdown-method-examples.html

@dennisameling
Copy link
Member Author

Closing in favor of #10228

@dennisameling dennisameling deleted the oauth1-quickfix branch July 4, 2021 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Tasks that relate to maintaining this Github repository CI Anything related to CI (Continuous Integration) cla-signed The PR contributors have signed the contributors agreement dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrading from 3.1.0 to 4.0.0-rc results in a fatal error
2 participants