Community
Participate
Working Groups
The standard mysql extension will be deprecated soon, so eclipse.org-common should use mysqli http://php.net/manual/en/book.mysqli.php
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
The mysql extension that was deprecated in PHP 5.5 is completely unsupported as of 7.0. I just installed Fedora 25. AFAICT there is no distribution of any version of PHP earlier than 7.0 (at least I couldn't find anything in any software repository that I trust). The obvious thing to do is to use a docker image. I'll sort that out. In the meantime, we should probably start putting plan to update to the mysqli together. FWIW, I've been tinkering with a function for doing queries that isolates the caller from the details of dealing with the mysql[i] APIs using closures (lambdas, anonymous functions). Below is the gist (there are some undefined variables, and some error checking is needed). function query($database, $sql, $variables, $each) { $mysqli = new mysqli($host, $user, $password, $database); $substitutions = array(); if ($variables) { foreach ($variables as $key => $value) { $substitutions[$key] = $mysqli->escape_string($value); } } if ($result = $mysqli->query(strtr($sql, $substitutions))) { while ($row = $result->fetch_assoc()) { $each($row); } $result->free(); } $mysqli->close(); } Callers can use this function like this: ... $sql = "select name, email from user where id='#id'"; $parameters = array('#id' => 'wbeaton'); $rows = array(); query('ipzilla', $sql, $parameters, function ($row) use (&$rows) { $rows[] = $row; // or you can do something interesting }); ... Something along these lines will allow us to make the updates that we'll need to make to our code base more future-proof. We probably need more functions than this; this is just the concept.
Thanks, Wayne. I think Chris has ideas about the database layer for eclipse.org-common, but I'm not sure. Wrapping the sql functions is definitely a prudent thing to consider.
(In reply to Denis Roy from comment #3) > Thanks, Wayne. > > I think Chris has ideas about the database layer for eclipse.org-common, but > I'm not sure. Wrapping the sql functions is definitely a prudent thing to > consider. +1 Me and Wayne spoke about this and it happens that Wayne started where I left off. I am going to include some of Wayne's work in my patch to see how this could work.
New Gerrit change created: https://git.eclipse.org/r/97383
(In reply to Eclipse Genie from comment #5) > New Gerrit change created: https://git.eclipse.org/r/97383 I created a patch to replace all mysql_ functions with their mysqli_ replacement in eclipse.org-common.git This patch requires the work that I am doing on the database classes since they include the required information to connect to each database: https://foundation.eclipse.org/r/1450 I added and modified Wayne query() function to my DB base class and it seems to work very well with $App->*_sql(). https://foundation.eclipse.org/r/#/c/1450/8/docker/_shared/php/eclipse-php-classes/system/DBConnectionBase.php This is still a work in progress and should not be merged. With this said, this seems to be working on my local environment but more testing is required to confirm this. I think the next step is to move this to our staging server for www.eclipse.org Matt, is it possible for you to scan eclipse.org and report who(projects/git repos) includes mysql_* functions in their code?
Sure thing: 2 cdo 2 cdt 1 community 1 donate 10 downloads 30 eclipse.org-common 12 eclipse 10 emf 1 go 5 legal 14 membership 15 modeling 1 newsgroups 6 org 53 projects 1 rtsc 3 tigerstripe 1 tm 64 webtools The upside is that almost 50% of those belong to the Foundation so we can update them 'whenever', but we need to let the projects know this is happening. Here's the timeline I'm thinking of: July 5-6: replace www-staging with a new host running the target php version and send a notice to the projects identified above that this change is coming September 5-6: actually replace the current hosts with newer ones. That gives us the summer to test, we can do the work in the staging branches and then just publish them to master when the change is finished. Sound ok? -M
I would like to propose that we migrate to PDO instead of mysqli. More information on this subject is available here: http://php.net/manual/en/mysqlinfo.api.choosing.php Is there any objections?
(In reply to Christopher Guindon from comment #8) > I would like to propose that we migrate to PDO instead of mysqli. > > More information on this subject is available here: > http://php.net/manual/en/mysqlinfo.api.choosing.php > > Is there any objections? I don't care. More specifically, nobody should have to care. Take a look at Comment #2 again. Your patch has a piece of my idea, but misses the most interesting part: if you make the sql() function take a function as a parameter, then the caller doesn't need to know or care if you're using mysqli, PDO, or whatever. It also results in tighter code. With your current implementation, the basic structure of a query doesn't really change: $result = $App->sql($database, $query, $variables); while ($row = $result->fetch_assoc($result)) { // or whatever you do in PDO if ($row['id'] == 'bob') { ... } } vs. $App->sql($database, $query, $variables, function($row) { if ($row['id'] == 'bob') { ... } }); That is, let the implementation of #sql fetch the rows. With this sort of method, we can easily change the implementation from using PDO to mysqli or whatever without requiring any changes from any code that uses it. If we're going to have to change every bit of code that uses the current implementation anyway, why not take the opportunity to make the change more future safe? Whether you clean up resources as you go or not is an implementation detail.
FWIW, I've implemented this for many of the project pages. When you decide to switch to PDO or mysqli, I'll update that single function and everything that uses it will just work. Note that the current implementation is a bit more complicated than it should be as it is designed to work using mysql on the server and mysqli on a development workstation. The implementation will become significantly simplier when we consolidate on a single database API.
(In reply to Wayne Beaton from comment #9) > (In reply to Christopher Guindon from comment #8) > > I would like to propose that we migrate to PDO instead of mysqli. > > > > More information on this subject is available here: > > http://php.net/manual/en/mysqlinfo.api.choosing.php > > > > Is there any objections? > > I don't care. More specifically, nobody should have to care. Take a look at > Comment #2 again. Your patch has a piece of my idea, but misses the most > interesting part: if you make the sql() function take a function as a > parameter, then the caller doesn't need to know or care if you're using > mysqli, PDO, or whatever. It also results in tighter code. I agree that the consumer of my API should not care if I use PDO, mysqli or even mysql. I understand the feature that you want to include in this project but we need to take a few step back right now and discuss the back-end before adding a database abstraction layer like I think you are describing. I can definitely see a your query() function included in my patch but we should decide how we are going to connect to our database before writing these nice to have features. After all, these functions will be written differently depending on the API we chose. First off, we need to protect access to our databases, db credentials & dbconnections. We also need to do a better job at re-using open database connections. We do care about if we are using Mysqli or PDO because I am responsible for opening new database connections. We also need to support legacy code. Our current implementation should continue to work until we decide on an EOL for mysql_ support. This is going to be a process. Here's what I am thinking (This plan still need to be discussed with IT): 1. Enable PDO in our API and continue to support mysql_ while we migrate our existing code to the new API. 2. Announce the EOL date for mysql_ API support at eclipse.org. 3. Remove the mysql php extension on or after the EOL date. > > With your current implementation, the basic structure of a query doesn't > really change: > > $result = $App->sql($database, $query, $variables); > while ($row = $result->fetch_assoc($result)) { // or whatever you do in PDO > if ($row['id'] == 'bob') { > ... > } > } > > vs. > > $App->sql($database, $query, $variables, function($row) { > if ($row['id'] == 'bob') { > ... > } > }); I am not sure if you are referring to my patch or the current code on production. Right now, the recommended way to write an sql query on eclipse.org is like this: $App = new App(); $sql = "SELECT f.friend_id FROM friends as f WHERE f.uid = " . $App->returnQuotedString($App>sqlSanitize('cguindon')); $result = $App->eclipse_sql($sql); print_r(mysql_fetch_array($result)); With my new "PDO patch", the recommended way to write an sql query would be like this: $Database = App::getDatabaseConnection('eclipse', 'slave'); $result = $Database->prepare("SELECT f.friend_id FROM friends as f WHERE f.uid = :user_uid"); $result->bind_param(':user_uid', 'cguindon'); $result->execute(); print_r($result->fetch()); As you can see, I could switch my DatabaseConnection class to use mysqli and my example/implementation would still work. I know it's not exactly the feature you are requesting but it's definitely a step in the right direction. Please note that I am using bind_param() on purpose in this example because unlike PDO, Mysqli requires that you use bind_param() to bind each parameter value to the prepared statement. With pdo, you could bind each parameter with an associative array in execute(). > > That is, let the implementation of #sql fetch the rows. With this sort of > method, we can easily change the implementation from using PDO to mysqli or > whatever without requiring any changes from any code that uses it. > > If we're going to have to change every bit of code that uses the current > implementation anyway, why not take the opportunity to make the change more > future safe? > > Whether you clean up resources as you go or not is an implementation detail. If I understand your comment correctly, you want us to create a database abstraction layer on top of our database connection class to give us the flexibility to upgrade to a different API in the future. (In reply to Wayne Beaton from comment #10) > FWIW, I've implemented this for many of the project pages. When you decide > to switch to PDO or mysqli, I'll update that single function and everything > that uses it will just work. I am definitely not against that idea! And as Denis did already mention, this would be the prudent thing to do. How about we use the work you already did for this feature? We could commit your work to eclipse.org-common instead of keeping it for project pages only? This should not impact the work that I am currently doing. We will also need to consider some time to write documentation for this new Database Abstraction layer API since we won't be able to refer users to the PDO/Mysqli documentation on php.net. > Note that the current implementation is a bit more complicated than it > should be as it is designed to work using mysql on the server and mysqli on > a development workstation. The implementation will become significantly > simplier when we consolidate on a single database API. I had the opportunity to experience this first hand today :) This is why I decided to propose that we chose PDO or mysqli instead of trying to support both!
(In reply to Wayne Beaton from comment #2) > FWIW, I've been tinkering with a function for doing queries that isolates > the caller from the details of dealing with the mysql[i] APIs using closures > (lambdas, anonymous functions). Below is the gist (there are some undefined > variables, and some error checking is needed). I think this is something that we could easily add in App(). For example, $App->query('eclipse', $sql, $variables, $each); I did not test this code but I assume it would be something like this: > > function query($database, $sql, $variables, $each) { > $mysqli = new mysqli($host, $user, $password, $database); First we would get a database connection using the work that I am currently focusing on: $Database = self::getDatabaseConnection('eclipse', 'slave'); > > $substitutions = array(); > if ($variables) { > foreach ($variables as $key => $value) { > $substitutions[$key] = $mysqli->escape_string($value); > } > } Then, we could do the substitution like this: $result = $Database->prepare($sql); $result->execute($variables); > > if ($result = $mysqli->query(strtr($sql, $substitutions))) { > while ($row = $result->fetch_assoc()) { > $each($row); > } > $result->free(); > } We could then process the results with: $data = $result->fetchAll(); foreach ($data as $row) { $each($row); } > > $mysqli->close(); This is not needed with my database connection class. I should take care of that for you. > } > > Callers can use this function like this: > > ... > $sql = "select name, email from user where id='#id'"; > $parameters = array('#id' => 'wbeaton'); > $rows = array(); > query('ipzilla', $sql, $parameters, function ($row) use (&$rows) { > $rows[] = $row; // or you can do something interesting > }); > ... > > Something along these lines will allow us to make the updates that we'll > need to make to our code base more future-proof. We probably need more > functions than this; this is just the concept.
New Gerrit change created: https://git.eclipse.org/r/110829
Created attachment 271275 [details] Differences between pdo, mysql_ and a new Database abstraction Layer API (In reply to Eclipse Genie from comment #13) > New Gerrit change created: https://git.eclipse.org/r/110829 I made some good progress on this today. Please note that this is a work in process but feedback is more than welcome. Right now, the priority should be on my new DatabaseConnection class. This class is basically a dispatcher for managing our database connections. I have a patch available for review on our Foundation Gerrit instance: https://foundation.eclipse.org/r/#/c/2002/ (We currently need to push these new classes to production before we can deploy changes to eclipse.org-common.) I also made available via Gerrit, an example of how we could use my new DatabaseConnection class in eclipse.org-common. The code is available here: https://git.eclipse.org/r/110829 My patch includes deprecation notices for every mysql_ function in App(). Also, I'm including a new Database Abstraction Layer prototype based off the idea from comment #2. Finally, I am attaching a screenshot of the differences between mysql_ (soon to be deprecated), PDO and our new Database Abstraction Layer (Database::query()). In summary, here's what's included in my screenshot: 1. SQL query with mysql_ API (Deprecated): require_once "../system/app.class.php"; $App = new App(); $sql = "SELECT f.friend_id FROM friends as f WHERE f.uid = " . $App->returnQuotedString($App->sqlSanitize('cguindon')); $result = $App->eclipse_sql($sql); $data = array(); while ($row = mysql_fetch_assoc($result)) { $data[] = $row; } print_r($data); Result: Array ( [0] => Array ( [friend_id] => 6101 ) [1] => Array ( [friend_id] => 6104 ) ) 2. SQL query with PDO: require_once "../system/Database.class.php"; $Database = Database::getDatabaseConnection('eclipse', 'slave'); $sql = "SELECT friend_id FROM friends WHERE uid = :user_uid"; $result = $Database->prepare($sql); $result->execute(array(':user_uid' => 'cguindon')); print_r($result->fetchAll(PDO::FETCH_ASSOC)); Result: Array ( [0] => Array ( [friend_id] => 6101 ) [1] => Array ( [friend_id] => 6104 ) ) 3. SQL query with Database::query(): This method should become the recommended process to execute an SQL query on eclipse.org. require_once "../system/Database.class.php"; $data = array(); $variables = array(':user_uid' => 'cguindon'); $sql = "SELECT friend_id FROM friends WHERE uid = :user_uid"; Database::query('eclipse', $sql, 'slave', $variables, function ($row) use (&$data) { $data[] = $row; }); print_r($data); Result: Array ( [0] => Array ( [friend_id] => 6101 ) [1] => Array ( [friend_id] => 6104 ) )
Quick update: I was working on one of our drupal sites this morning with the new database class that I am currently working on. I wanted to see how my code would behave under different environments. I quickly noticed the same error in our logs: PHP Fatal error: Cannot redeclare class Database in eclipse.org-common/system/Database.class.php on line 17 The problem here is that Drupal also declare a Database class. The quick fix would be the rename our Database class but it's NOT the right thing to do. We will run into this problem again in the future. We need to start using namespaces in our code for portability. I created a bug last year that discuss this idea since we encountered this issue with classes defined in the PMI. For more information on the subject please see: Bug 496514 - PHP sites maintained by the EF should follow recommendations from the Framework Interop Group https://bugs.eclipse.org/bugs/show_bug.cgi?id=496514 The main blocker (beside resources/time) is that we still have some servers running PHP 5.2. Namespaces became available in php 5.3 (http://php.net/manual/en/language.namespaces.rationale.php). Since servers with php 5.2 include code from eclipse.org-common, we need to upgrade them before we can include namespaces in eclipse.org-common. FWIW - I added namespace in my database class and everything seems to be working with Drupal. Please note that I didn't update my patch on Gerrit yet.
(In reply to Christopher Guindon from comment #15) > Quick update: > > I was working on one of our drupal sites this morning with the new database > class that I am currently working on. I wanted to see how my code would > behave under different environments. > > I quickly noticed the same error in our logs: > > PHP Fatal error: Cannot redeclare class Database in > eclipse.org-common/system/Database.class.php on line 17 > > The problem here is that Drupal also declare a Database class. The quick fix > would be the rename our Database class but it's NOT the right thing to do. > We will run into this problem again in the future. > > We need to start using namespaces in our code for portability. > > I created a bug last year that discuss this idea since we encountered this > issue with classes defined in the PMI. > > For more information on the subject please see: Bug 496514 - PHP sites > maintained by the EF should follow recommendations from the Framework > Interop Group > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=496514 > > The main blocker (beside resources/time) is that we still have some servers > running PHP 5.2. Namespaces became available in php 5.3 > (http://php.net/manual/en/language.namespaces.rationale.php). > > Since servers with php 5.2 include code from eclipse.org-common, we need to > upgrade them before we can include namespaces in eclipse.org-common. > > FWIW - I added namespace in my database class and everything seems to be > working with Drupal. Please note that I didn't update my patch on Gerrit yet. I took a look at trying to get this to work with PHP-52. After spending an hour on this, I was able to get it to work by removing my "namespaces" but it became clear to me that this was the wrong solution. We are adding a bunch of exception for servers that we plan on removing from our stack very soon. If we decide to go down this road, we will need to re-visit our code once our php 5.2 servers are removed from our infrastructure. I am putting further developpement for this on hold until we no longer have any php 5.2 servers in service. Also, php 5.2 is officially deprecated and no-longer supported while the mysql extension was only removed in PHP 7. We currently do not have any servers running php 7. I think that it will be important to leverage namespaces for this project because this code will run under different frameworks and CMS such as Drupal. We need to get this right before we ask our projects to update their code.
I spoke to Matt today about this and we would like to move all our web-front end on dev.eclipse.org to our www-vm nodes in Q1 2017. If we can do this, we will be able to stop supporting php 5.2 in our core code base and move forward with this new patch in Q2 2017.
(In reply to Christopher Guindon from comment #17) > I spoke to Matt today about this and we would like to move all our web-front > end on dev.eclipse.org to our www-vm nodes in Q1 2017. > > If we can do this, we will be able to stop supporting php 5.2 in our core > code base and move forward with this new patch in Q2 2017. I assume that I was referring to Q1 2018. Regardless, this hasn't been done. I created a bug to address this problem which should allow us to move forward with this bug.
This issue has been migrated to https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/164.
