Skip to content

Added some method inside JDatabaseDriver to create database and alter character set #1261

Closed
wants to merge 7 commits into from

5 participants

@gpongelli

Added two public method inside JDatabaseDriver:

  • alterDbCharacterSet to change database's character set
  • createDatabase to create a new database

AlterDbCharacterSet needs database name as parameter, createDatabase
needs a JObject, similar to that used inside CMS' "initialise" function,
and a bool to set UTF8 database's support.

They use two protected method to obtain correct query in database's
supported syntax:

  • getAlterDbCharacterSet
  • getCreateDatabaseQuery

Protected method inside JDatabaseDriver return query for MySQL and
MySQLi syntax, this commit contain also overridden version for
PostgreSQL database.

There are tests only for protected methods inside MySQL, MySQLi and
PostgreSQL test classes.

gpongelli added some commits Jun 7, 2012
@gpongelli gpongelli Added two public method inside JDatabaseDriver:
- alterDbCharacterSet to change database's character set
- createDatabase to create a new database

AlterDbCharacterSet needs database name as parameter, createDatabase
needs a JObject, similar to that used inside CMS' "initialise" function,
and a bool to set UTF8 database's support.

They use two protected method to obtain correct query in database's
supported syntax:
- getAlterDbCharacterSet
- getCreateDatabaseQuery

Protected method inside JDatabaseDriver return query for MySQL and
MySQLi syntax, this commit contain also overridden version for
PostgreSQL database.

There are tests only for protected methods inside MySQL, MySQLi and
PostgreSQL test classes.
0325d8f
@gpongelli gpongelli Comments correction. 5a75e70
@elinw elinw commented on an outdated diff Jun 10, 2012
libraries/joomla/database/driver.php
+ *
+ * @since 12.2
+ * @throws RuntimeException
+ */
+ public function createDatabase($options, $utf = true)
+ {
+ if (is_null($options))
+ {
+ throw new RuntimeException('$options object must not be null.');
+ }
+
+ $this->setQuery($this->getCreateDatabaseQuery($options, $utf));
+ return $this->execute();
+ }
+
+
@elinw
elinw added a note Jun 10, 2012

Extra blank line here. 443 and 444 also have white space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elinw elinw commented on an outdated diff Jun 10, 2012
libraries/joomla/database/driver.php
+ * @since 12.2
+ */
+ protected function getCreateDatabaseQuery($options, $utf)
+ {
+ if ($utf)
+ {
+ $query = 'CREATE DATABASE ' . $this->quoteName($options->db_name) . ' CHARACTER SET `utf8`';
+ }
+ else
+ {
+ $query = 'CREATE DATABASE ' . $this->quoteName($options->db_name);
+ }
+
+ return $query;
+ }
+
@elinw
elinw added a note Jun 10, 2012

This line 574 has white space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gpongelli

Added style fixes and checks for createDatabase method.

@stefanneculai

I think it should be &$columns - there is a coding style error.

Doc comment for var $columns does not match actual variable name &$columns at position 1

@gpongelli

Fixed.

Eng. Gabriele Pongelli

@realityking realityking and 1 other commented on an outdated diff Jul 18, 2012
libraries/joomla/database/driver.php
+ * @param JObject $options JObject coming from CMS' "initialise" function to pass user
+ * and database name to database driver.
+ * @param boolean $utf True if the database supports the UTF-8 character set.
+ *
+ * @return string The query that creates database
+ *
+ * @since 12.2
+ * @throws RuntimeException
+ */
+ public function createDatabase($options, $utf = true)
+ {
+ if (is_null($options))
+ {
+ throw new RuntimeException('$options object must not be null.');
+ }
+ elseif (!isset($options->db_name) || empty($options->db_name))
@realityking
Joomla! member
realityking added a note Jul 18, 2012

I think this can be simplified to just the empty() check.

@gpongelli
gpongelli added a note Jul 19, 2012

Changed, I'll commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@realityking realityking commented on an outdated diff Jul 18, 2012
libraries/joomla/database/driver.php
+ * @return string The query that creates database
+ *
+ * @since 12.2
+ * @throws RuntimeException
+ */
+ public function createDatabase($options, $utf = true)
+ {
+ if (is_null($options))
+ {
+ throw new RuntimeException('$options object must not be null.');
+ }
+ elseif (!isset($options->db_name) || empty($options->db_name))
+ {
+ throw new RuntimeException('$options object must have db_name set.');
+ }
+ elseif (!isset($options->db_user) || empty($options->db_user))
@realityking
Joomla! member
realityking added a note Jul 18, 2012

Same as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@realityking realityking commented on an outdated diff Jul 18, 2012
libraries/joomla/database/driver.php
@@ -397,6 +418,38 @@ public function __construct($options)
abstract public function connected();
/**
+ * Create a new database using information from $options object, obtaining query string
+ * from protected member.
+ *
+ * @param JObject $options JObject coming from CMS' "initialise" function to pass user
@realityking
Joomla! member
realityking added a note Jul 18, 2012

We don't actually need a JObject here. A generic object will do. Also please don't base you comment based on CMS use but be generic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gpongelli

Committed changes.

Gabriele.

gpongelli added some commits Jul 21, 2012
@ianmacl
ianmacl commented Aug 14, 2012

So what's the point of public function alterDbCharacterSet($dbName) ?

@gpongelli

Hi Ian,
I added that method because it's used during CMS installation, as createDatabase one, to complete installation on Postgresql.
These methods were born after a private discussion with Andrew, who point me adding these methods here.

Gabriele.

@elinw
elinw commented Oct 18, 2012

Can you update your branch please?

@gpongelli

updated HERE, I'll close this pull.

@gpongelli gpongelli closed this Oct 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.