Add support for mysql 5.6.x.

[yamilovs]

It will fix this warning: "Warning: Using a password on the command line interface can be insecure."

[Nyholm]

I like this change. Though, I've not tested it yet. 👍

[Nyholm]

I think this could be rewritten to avoid array merge. It makes the code easier to read.

[yamilovs]

I can rewrite it like
$cnfParams += array('key' => 'value')
but this feature was provided in php 5.4. I should change composer.json to requre new version of php (old configuration is "php": ">=5.3.2") it may cause some problems with BC. If it is not a problem, i'll do this.

PS. i doesn't want to add some params like this:
$cnfParams[key] = "value"; $cnfParams[another_key] = "another_value" // etc

[dizda]

FYI, 5.3's support has just been dropped via #109

[yamilovs]

Ok i rewrited it with array addition

[yamilovs]

And i found one issue, that mysql.cnf configuration file was dumped with all requsites to backup file. I think it is not good. I'll try to fix this soon.

[yamilovs]

Hello!
I made some changes in MySQL dump structure. Some constructor refactoring, and now removing file with connect requsites from database dump. Can u check it?

[yamilovs]

@Nyholm
Hello, Tobias. Do u have any progress on this PR? Thanks in advance.
PS. We want to use this bundle on our PHP7 project, but it doesn't work correctly on MySQL 5.6. Of course i can create a fork, but i think it is not a good idea =)

[Nyholm]

Sorry for being slow on getting back on this PR. I'll test and review it this week.

PS. We want to use this bundle on our PHP7 project, but it doesn't work correctly on MySQL 5.6. Of course i can create a fork, but i think it is not a good idea =)

Please make PRs and I (or someone) will review it. We want to support PHP7.

[dizda]

Hello, personally we don't have any problems with PHP7+MariaDB and this bundle.
The issue might be related directly to MySQL5.6.

Just one question, why do you have the mysql.cnf in your backup files? I don't get it.

[yamilovs]

Hello, @dizda. mysql.cnf was in 60af855 commit. I fixed this issue in 8e231ee.

[Nyholm]

What if an exception is thrown somewhere. I do not what to risk that a configuration file is lying around. Let's use a try/catch and make sure 'removeConfigurationFile' always is running.

[yamilovs]

If an exeception will be thrown, it will be catched in manager/BackupManager->execute() function and all backup folder will be cleared.

[Nyholm]

okey, good.

[Nyholm]

Could you rebase on master so Travis will have a change to show green?

[Nyholm]

How about rewriting it to this?

 if ($this->params['db_user']) {
  if ($this->params['db_password']) {
    $cnfParams = array(
      "password"  => $this->params['db_password'],
      "host"      => $this->params['db_host'],
      "port"      => $this->params['db_port']
    );
  }

  $cnfFile  = "[client]\n"; 
  $cnfParams['user']  = $this->params['db_user'];
}

This would be easier to read, right?

[Nyholm]

Also, is the logic here correct? Should we add host and port only iff we have a password?

[yamilovs]

This logic was already there before i started working on it =)
And your version of rewriting is more dificult to read for me =) I think its something minor, but if you insist, I'll rewrite it.

[yamilovs]

what do you think about this variant?

        $cnfFile = "[client]\n";
        $cnfParams = array();
        $configurationMapping = array(
            'user'      => 'db_user',
            'password'  => 'db_password',
            'host'      => 'db_host',
            'port'      => 'db_port',
        );

        foreach ($configurationMapping as $key => $param) {
            if ($this->params[$param]) {
                $cnfParams[$key] = $this->params[$param];
            }
        }

        if ($cnfParams) {
            foreach ($cnfParams as $key => $value) {
                $cnfFile .= "$key = \"$value\"\n";
            }

            $this->filesystem->dumpFile($this->getConfigurationFilePath(), $cnfFile, 0600);
            $this->auth = sprintf("--defaults-extra-file=\"%s\"", $this->getConfigurationFilePath());
        }

It's more correct logic, but i think it's also hard to read.

[Nyholm]

Ah =) okey.

No, I do not insist. I'll want your opinion. I dislike array_merge and the plus operator in this case because it could be avoided.

How about this:

 if ($this->params['db_user']) {
  $cnfFile  = "[client]\n"; 
  $cnfParams['user']  = $this->params['db_user'];

  if ($this->params['db_password']) {
    $cnfParams['password'] = $this->params['db_password'];
    $cnfParams['host']     = $this->params['db_host'];
    $cnfParams['uporter']  = $this->params['db_port'];
  }
}

[Nyholm]

Yes, Yours is way better. But for clarity I would like if (!empty($cnfParams)) {

[yamilovs]

Yep, ok) I'll do it like this.

[yamilovs]

Ok, guys, all done!

[Nyholm]

@dizda Merge at will!

[dizda]

Ok, much more clear and secure! Thanks.

Last thought, do you remove the file mysql.cnf in the case when an Exception happened?
Because that would be un-secure to leave this file on the server.

[yamilovs]

If exception occurs, it will be handle with execute() function in manager/BackupManager, that remove all backup directory with mysql.cnf file too.

[dizda]

Sweet, let's do it then!

[dizda]

Thanks, https://github.com/dizda/CloudBackupBundle/releases/tag/3.4.0 released.