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

Rewrite for 6.0 #624

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

Rewrite for 6.0 #624

wants to merge 32 commits into from

Conversation

alessandro-fazzi
Copy link
Member

@alessandro-fazzi alessandro-fazzi commented Dec 26, 2021

Preliminary note: the commit history hasn't been kept: it had no sense at all. Unfortunately I'm not able to fully squash the history because of a "backport" I've made in the middle causing too much conflicts during history rewrite attempts. Shit happens: I'll keep it easy :)

Alpha version for testing

If you have other versions installed, wipe it with gem uninstall wordmove, then

gem install wordmove --version '6.0.0.alpha.8'

The alpha version is shipped for internal testing; everyone feels free to be courious, but I'd advice to test it against staging/testing environments and anyway be wise about your backups.

Why such a big refactor?

What was wrong

  • wordmove is about to turn 10 years old
  • code architecture was weak: way too complex to accomplish to run a procedure
  • code was a bit old-fashioned: naive meta-programming, inheritance abuses
  • difficult to read

thus

  • hard to get on and contribute
  • hard to debug and fix
  • hard to test
  • hard to maintain: a lot of time
  • harder to add/modify behaviours

Which goals?

Since Wordmove is almost doing what it was created to do for almost 10 years, I can't see huge room nor huge urgency to expand its functionalities. But it still has it's place in the "market", thus I'd like to keep it alive.

In order to keep it alive I had some goals:

  • easier to read, write and test
  • easier to be contributed from the community
  • better documentation
  • have less frustration and more fun when I open it in my editor :)
  • study some interesting gems and patterns

How does this PR accomplish those goals?

New command line implementation

the command line implementation has been re-implemented using https://dry-rb.org/gems/dry-cli/. dry-rb is minimal, class based and heavily object oriented

Service objects all the way

All the core logic is now organised into service objects; we have two organisers for SSH Wordmove::Organizers::Ssh::Push and Wordmove::Organizers::Ssh::Pull and two for FTP Wordmove::Organizers::Ftp::Push and Wordmove::Organizers::Ftp::Pull in charge of orchestrating all the single actions needed to accomplish the procedure. Reading the organisers should immediately give you the idea of what the code is accomplishing to do.

Let's look for example into lib/wordmove/organizers/ssh/push.rb

# [...]

        with(
            cli_options:,
            global_options: movefile.options[:global],
            local_options: movefile.options[:local],
            remote_options:,
            movefile:,
            guardian: Wordmove::Guardian.new(cli_options:, action: :push),
            logger:,
            photocopier: Photocopier::SSH
                          .new(ssh_opts)
                          .tap { |c| c.logger = logger }
          ).reduce(actions)
        end

        def self.actions
          [
            Wordmove::Actions::RunBeforePushHook,
            Wordmove::Actions::FilterAndSetupTasksToRun,
            reduce_if(
              ->(ctx) { ctx.wordpress_task },
              [Wordmove::Actions::Ssh::PushWordpress]
            ),
            iterate(:folder_tasks, [Wordmove::Actions::Ssh::PutDirectory]),
            reduce_if(->(ctx) { ctx.database_task },
                      [
                        Wordmove::Actions::SetupContextForDb,
                        Wordmove::Actions::Ssh::DownloadRemoteDb,
                        Wordmove::Actions::Ssh::BackupRemoteDb,
                        Wordmove::Actions::AdaptLocalDb,
                        Wordmove::Actions::Ssh::PutAndImportDumpRemotely,
                        Wordmove::Actions::Ssh::CleanupAfterAdapt
                      ]),
            Wordmove::Actions::RunAfterPushHook
          ]
        end

where it's evident what's the procedure being executed; the list of actions will be executed in order, and every action will have access to same context, where all the needed state (almost configurations) resides.

This way, and thanks to the new design, development and bug fixes should be naturally more focused.

Testing

Testing single actions is now more straight. The coverage has been preserved and slightly improved.

The CI, moved on GitHub actions, is also a bit more polished and for sure faster.

Documentation

Now the code has an extremely wider in-code documentation coverage. When 6.0.0 will be out of pre-release, you'll find updated developer's documentation at https://www.rubydoc.info/gems/wordmove.

Not a complete rewrite still

Some part of the legacy code have been loosely adapted to work into the new design patter; I'm not happy with this surviving code, but I'll keep it as it is. Time investment has already grown too much.

Features

This release is not about features, but

  • --debug flag's now more useful when you actually are dealing with bugs (try it out and you'll see a lot of info about the currently executed actions)
  • the CLI UI has been updated to be more expressive and useful to read (still simple like I like it, tho)
  • Now it's possible to disable secrets masking on STDOUT by setting the environment variable WORDMOVE_REVEAL_SECRETS=1, e.g: WORDMOVE_REVEAL_SECRETS=1 wordmove pull -d -e stging. Just please don't use this feature to report your STDOUT in GitHub's issues.
  • dependencies has been updated
  • run on current stable ruby version (required too)

Maybe for the beta release also the ruby version will be bumped. I've still to check if the update to Ruby 3 would be easily affordable.

Bugfixes

Dropped features

  • Now only the .env file is supported. Variations like .env.development are not supported as it stands in the alpha version.
  • The default global.sql_adapter has been dropped. This is a braking change and means that wp-cli will be a required peer dependency for >=6.0.0. Wiki docs will be updated accordingly at release time. The key in the movefile.yml is still required, so you need to have
global:
  sql_adapter: wpcli

Breaking changes

  • Being considered an edge, now wordmove requires the latest ruby stable version: 3.1.0. Since wordmove is written in ruby but it's a tool used by PHP developers, I'm committed to take it as edge as possible regarding the required interpreter version. Any user should have problem always running the current stable.
  • wpcli is a mandatory peer dependency. You need it or you'll have to stick to wordmove <6.0
  • Big changes in movefile.yml: local.database key and local.vhost key are no more required and considered a syntax error. doctor command is aware of the change. More on this in a later paragraph (see Wpcli_all_the_way #625 )
  • global.sql_adapter, stil being mandatory, now supports only wpcli.

Thus remember to update you movefile.yml accordingly. The wiki documentation will be updated lately so you'll be on your own until then.

Configuring the local section: news and drops

From now on this will be your local section in movefile.yml:

local:
  wordpress_path: /users/fuzzy/dev/cosy_site

This sounds - and actually is - really cool, but you have to be aware of a few things.

local.database.mysql_options and local.database.mysqldump_options are dropped. Now we rely on wp-cli in order to dump and import the database. You can still configure wp-cli in order to achieve some of the previous configuration. In order to configure wp-cli you can follow the official docs https://make.wordpress.org/cli/handbook/references/config/#config-files, but here come some examples of a wp-cli.yml file

db export:
  exclude_tables: wp_posts,wp_options
  hex-blob: true
  no-create-info: true
  verbose: true
  # and even more at https://developer.wordpress.org/cli/commands/db/export/

db search-replace:
  skip_tables: wp_stats_*
  # and more at https://developer.wordpress.org/cli/commands/search-replace/

db import:
  # and on and on

Keep in mind that we already always use some flags when wrapping wp-cli commands:

  • search-replace: --precise, --regex, --regex-delimiter="|", --skip-columns=guid, --all-tables
  • all commands: --allow-root

Keep in mind you are able to overwrite them creating hard to track misbehaviours.

Also local.database.port and local.database.charset are dropped since they are managed differently in wp-config.php and anyway automatically managed by wp-cli.

Spread some single quote love

Remove metaprogrammed method definitions from deployer Base

Here start the long way of a refactor. I've simply explicitly
defined the methods, removing the metaprogramming pain.

Bloatet and verbose, but easy to follow and to read.

WordpressDirectory::Path module no more in a separate file

Rename type variable to folder, and change `attr_accessor`s to `attr_reader`s

Add tests for WordpressDirectory

Extract directory methods into modules

Freeze `options` inside Deployer::Base

instead of cloning theme; cloning was useless
and it consumes unnecessary memory.

Rename `name` variable into `config_file_name` in Movefile`

Refactor Movefile class

Major changes:

* `initialize` now requires whole `cli_options`,
  not just the config part. This changes the internal
  API, but free the implementor from passing `cli_options`
  to all subsequent public methods such as
  `load_dotenv`, `environment` and `secrets`.
  Once the object is created, it has all the information
  it needs to do its work.
* now all the classes below CLI want deep symbolized
  options. So options `Hash` from the movefile and cli
  options have to be symbolized in order to
  guarantee to be correctly managed.
  The mix of `Hash`es with string keys and with symbol
  keys was confusing, hard to understand and to use
  both in the logic and in tests
* now `Movefile.fetch` is coupled within `new`,
  so once instantiated the object, you'll have access
  to `object.options` to retrieve all the options
  from the movefile file.

`load_dotenv` is now a private method. WARNING: removing support for `.env.envName` files

Start to write the first organizer

Actions::Ssh::Concerns

Require all actions

Rename concerns in Helpers and do not use context as argument

Fix typo

Partial implementation for live testing + first debug

Rename and move common ssh actions and helpers

add photocopier actions

Refactor Hooks to fit service object pattern and create hooks pull actions

Thoughts in comments

WordpressDirectoryHelperMethods are now class methods to fit into actions

Pull Organizer now also manages hooks

Making tests pass, mostly with skips during the refactor :)

Before_actions can't stop actions; move to reduce_if

Moved and refactored other helpers

Written all the pull directories actions

Ssh::Pull organizer now pretends one less param

Wordmove::CLI.wordpress_options is now a class method

GetDirectory moved under Ssh namespace and generalized

Now it's more complex, but able to handle pulling
any directory for an organizer

Introducting FilterAndSetupTasksToRun and refactor

Thanks to this action we're able to remove a lot of
complexity from the organizer, since the Guardian
is called one time ahead of all the actions
sequence, no more one time for each action

Clean up actions array in Pull organizer

Update Guardian initialize signature with more clear param

Commenting code to keep track of the refactored methods

Add Actions::DeleteCommandFile

Add Actions::RunLocalCommand

Add Actions::Ssh::RunRemoteCommand

Implement WpcliAdapter::PullDb Action/Organizer

update movefile spec

Call the Pull interactor from cli. FTP still to be done

Ruby version 2.7.1

Refactor pull organizer removing action+organizer class

Moving Rubocop to Ruby 2.7.1

Rubocop: no more prefere double quotes

First turn of double to single quote conversion :)

Add some comments to helpers

Keyword argument for `uncompress_command` helper

Add dry-configurable and create a configurable object for db paths

Generalize `SetupContextForDB` and introduce db_paths into context

Use the new `db_paths` in organized pull actions

Generalize `CleanupAfterPull` to `CleanupAfterAdapt`

`ImportRemoteDump` renamed to `PutAndImportDumpRemotely` and refactored

s
Fix bug in AdaptRemoteDb

Undefined method `waring` was called on logger

Fix wrong signature on method call in AdaptRemoteDb

Warn when db adapt is skipper

SetupContextForDb won't run unless necessary

Update some log messages for better clarity

Fix mu-plugins key in mu_plugins. Fixes #619

Please the rubocop

Remove FTP support and cleanup as consequently

Re-implement #597 in this branch

Improve wpcli_search_replace_command with argument check

Add yard documentation

Fix bug in FilterAndSetupTasksToRun action

Was getting key by string on a symbolized hash

Migreate from Thor do dry-cli

Improve UX a bit w/ better log messages

Remove unused WordpressDirectory::HelperMethods module

Re-enable --debug flag

Update version: will stick to a pre-release

Update scaffold and namespace for organizers

WIP: re-implement FTP support

WIP: re-implement FTP support

Fix typo and reword a variable

WIP: re-implement FTP support

WIP: re-implement FTP support

WIP: re-implement FTP support

WIP: re-implement FTP support

WIP: re-implement FTP support

WIP: re-implement FTP support

WIP: re-implement FTP support

WIP: re-implement FTP support

Update get_file_spec

Update hook_spec

Update rspec config

Add adapt_local_db_spec

Various cleanups, typos, rubocop disables

Fix Wordmove::Movefile by removing - never triggered - wrong logic

Update deps and please the (new) rubocop
@alessandro-fazzi alessandro-fazzi force-pushed the rewrite_for_6.0 branch 14 times, most recently from ed376f0 to 12fe790 Compare December 26, 2021 17:07
We had scenarios where failures were not interrupting
the procedure even if expected.
I think it's better to warn on such failures than
stop the whole procedure since they are not real
blockers.

I know it could be risky to leave undeleted files
on the remote, but probably would be worst to
leave an incomplete procedure during certain
stages
It's worth to note that we're fixing this problem
with the cost of slowing down the operation.

For me it's solid and it's an acceptable tradeoff,
but on large databases this could visibly decrease
the overall procedure execution speed
@alessandro-fazzi
Copy link
Member Author

Thanks @nlemoine for this ab39a4a

Fixed in alpha.8

@electricarts
Copy link

Sorry for the maybe stupid questions from a pixel pusher:

Since from version 6 mysql and mysqldump options are no longer available, then the connection to the (local) database via socket also no longer works, right?

Currently I use the following entries in the movefile.yml in connection with local (flywheel):

name: local
user: root
password: "root"
host: localhost
mysqldump_options: '--column-statistics=0 --socket "/Users/mario/Library/Application\ Support/Local/run/w_2HLYrYO/mysql/mysqld.sock"'
mysql_options: '--socket "/Users/mario/Library/Application\ Support/Local/run/w_2HLYrYO/mysql/mysqld.sock"'

@alessandro-fazzi
Copy link
Member Author

alessandro-fazzi commented Jan 10, 2022

Hello @electricarts :)

Thnaks for the inquiry. That's not correct or at least that's not intended :P

Flags to the underlying mysql and mysqldump commands are still possible to be proxied by configuring wp-cli. I wrote something about it in a paragraph of this PR, but for sure a more structured documentation will follow the future release.

Moreover I think that using wp-cli to do BD operations, as far as your wordpress is able to connect to the DB, then you should be able to go configuration-lessly: wp-cli reads configurations directly from your wordpress installation.

That said I'd be more than happy to help you troubleshooting the specific flywheel scenario if you'd be able to perform some alpha or - soon - beta testing: I'm very interested in retaining support for such a widespread stack.

Let me know and feel free to open a discussion if you need to chat about the matter :)

P.S.: I've not yet tested the alpha against nothing but a plain standard WP install into a directory

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants