? Failure

User tests: Successful: Unsuccessful:

avatar jms2win
jms2win
24 Jan 2014

On request of Bakual, I have more the PR 2822
#2822

from master to staging branch

avatar jms2win jms2win - open - 24 Jan 2014
avatar Bakual
Bakual - comment - 24 Jan 2014

Changed title of the PR to the title of the original PR.
Also adding here the description of original PR:

In case of a manual upgrade from Joomla 2.5.17 to J3.2.1, it happens that some tables or columns are not present in the DB before that the DB / Fix is applied and therefore may cause DB Errors.

To allow using the Database / Fix functionality, it is important that this intercept all kind of DB errors.

This is why we added a try/cath on the loadObject that allow have access to the Extension Manager / Database / Fix action.

Comment history can be seen in original PR.
Trackeritem: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33177

avatar jms2win
jms2win - comment - 24 Jan 2014

Sorry but I don't understand what you want that I do and where.

The title of which PR do you want that I change ?
The exact number please.

I think that I have clearly identified in the new PR that I have created at your request and it makes reference to this one (PR).
So all the information can be retrieved and also from the Bugtracker as this one also make reference to the new PR

If you want to add comments in the new PR to make them more clear, this is possible.

Do you want that I add a comment in the Bugtracker to mentioned the new PR, this is also possible but you can do it also.

Sorry but I am not fluent with PR.

avatar brianteeman brianteeman - change - 24 Jan 2014
Status New Pending
avatar Bakual
Bakual - comment - 24 Jan 2014

@jms2win Nothing to do for you, all is good now. Thanks for that!
I just added some notes here (this is the new PR) and changed the title (myself) to make it clearer. Sorry for that confusion.

Do you want that I add a comment in the Bugtracker to mentioned the new PR, this is also possible but you can do it also.

Yep, that would be great!

avatar brianteeman brianteeman - change - 2 Sep 2014
Category SQL
avatar brianteeman
brianteeman - comment - 2 Jan 2015

In the original issue @mbabker stated

Again, you're completely masking an error situation here. I get what you're trying to do, but again, just blindly ignoring the error is not the right solution to me.

It's been a year since the original PR was made. What is the status on this. If it is not going to be accepted then this should be closed


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/2839.
avatar brianteeman brianteeman - change - 2 Jan 2015
Status Pending Needs Review
avatar jms2win
jms2win - comment - 2 Jan 2015

This is a regression bug between J2.5 and J3.x.
In J2.5, it does not stop when you have duplicated records or columns or any error.
The objective of the extension manager / database => Fix button is to fix the DB structure and not stop at the first duplicate stuff that would be encounter.
In J2.5, this does not cause any error because it has a return code that is managed after in the process.

In J3.3, it not possible to manage the return code as it throw an exception that stop the processing.
The objective is to make the code compatible with J2.5 and convert the throw exception into the "soft" error that is manage alter by the function.
If you don't want to do that, you need to review (refactor) totally this functionality to process the error correctly with throw exception instead of using the return code as in J2.5.

avatar brianteeman
brianteeman - comment - 2 Jan 2015

Can you provide instructions for how to replicate the issue and test the
proposed solution please

On 2 January 2015 at 08:01, jms2win notifications@github.com wrote:

This is a regression bug between J2.5 and J3.x.
In J2.5, it does not stop when you have duplicated records or columns or
any error.
The objective of the extension manager / database => Fix button is to fix
the DB structure and not stop at the first duplicate stuff that would be
encounter.
In J2.5, this does not cause any error because it has a return code that
is managed after in the process.

In J3.3, it not possible to manage the return code as it throw an
exception that stop the processing.
The objective is to make the code compatible with J2.5 and convert the
throw exception into the "soft" error that is manage alter by the function.
If you don't want to do that, you need to review (refactor) totally this
functionality to process the error correctly with throw exception instead
of using the return code as in J2.5.


Reply to this email directly or view it on GitHub
#2839 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar mbabker
mbabker - comment - 2 Jan 2015

In theory this solution works. I'm still not convinced that this is "right"; nowhere is the error logged or the user notified of it (unless you've got debug mode enabled, which shouldn't be the case 99.9% of the time), and that part concerns me as much as code that explicitly sets error_reporting(0);. All errors should be catchable in some form for end users and admins to review.

2.5 used very poor quality error handling; 3.x in general has moved toward handling PHP Exceptions. In doing so, there's a lot of code to update to decouple dependencies on the legacy error handler and to properly handle thrown exceptions. Does every error need to end the app's request cycle? No (and this is one instance), but there should be some sort of awareness of a problem. Even something as simple as a counter that enqueues a message when the check is complete stating "X queries had errors while performing the check".

avatar jms2win
jms2win - comment - 2 Jan 2015

@mbabker, normally this Fix Database should NEVER stop when processing the fix of the DB.
If you have an extension that for example perform an INSERT of a record that already exists, this stop that is not acceptable.
The Fix Database should fix as much as possible.
In addition, it may happen that some fix are related to other dependencies that would be solved later and therefore as you stop, you didn't give any chance to execute the fix several time.

Normally, a good fix tool you should execute ALL the fix statement that are OK and not stop at the first error like a compiler that reports you all the errors in your code and not stop at the first error.

The problem is that the "schema" does not analyse and parse all the possible MySQL statements that may result in failure.
In Jms multisites, we have implemented an advanced Fix Database to parse more SQL statement and perform better check than in the native Joomla that is not the case.
So our proposal is to give the opportunity to fix as much as DB error as possible and not stop at the first error. This is a problem of logic in J3.3 that didn't occurred in J2.5 as the "soft" error was not stopping and that the other fix statement were processed correctly.

avatar mbabker
mbabker - comment - 2 Jan 2015

Like I said, the fix in theory in fine. I'm still concerned by the fact there is no notification of the error to anyone, that's all. No log message, no on-screen notification, no indication that something behind the scenes isn't quite right. In 2.5, you almost had to explicitly check for an error in a query; with the exception logic now your code needs to be more error aware or you get circumstances like this where it goes uncaught and bubbles up the application stack to our global handler.

So to be clear, I'm fine with catching the exception and not aborting execution here. I'm not OK with errors being masked, not just in this instance but in general.

avatar brianteeman
brianteeman - comment - 2 Jan 2015

Instead of masking the errors which as Michaels says it is not a nice thing to do why dont you contribute your
In Jms multisites, we have implemented an advanced Fix Database to parse more SQL statement and perform better check than in the native Joomla that is not the case.

avatar jms2win
jms2win - comment - 2 Jan 2015

@mbabker, I agree that we could report more information to the user but I didn't check the difference between 2.5 and 3.3 for this point. Based on my memory, the code that is present in J3.3 comes from J2.5 without any modification and therefore, if there are some reporting, perhaps that they are still there (I don't know and didn't investigate that)
If you think that more display message should be reported, up to you to add them.

@brianteeman In JMS we are doing fare more SQL parsing and for example, we are also able parsing the complex insert (multiple insert in one insert) that we analyse and test to identify the parts of the insert that might be missing or causing an error.
We also processed other cases that are no implemented in the core Joomla like views.
To give you an idea of the complexity, the code present in JMS, it is 3 times the code present in Joomla and that inherit of the schema present joomla. So in terms of complexity, you need to multiply by 4 the current Joomla implementation. If you can have access to the JED, you can get JMS and have a look in the "maintenance" menu that implement an alternative solution to the Joomla Fix DB that catch all the errors, provide a full reporting of the error and the reason of the error with also the files that cause the error and the line in the file that cause the error. At the difference of the current Joomla Fix Database that just mention the number of errors, we report the detail and the reason of the error (that is not the case in Joomla).
So, with JMS, it is possible to know which update file has an error.
Our Fix database is also able to work on any 3rd parties extensions that does not publish the update script.
JMS Fix Database is able to parse the manifest file to retrieve the MySQL installation scripts or manifest XML queries or compare the structure with another website DB to verify the consistency of the DB and deduct the ALTER Table that must be generated to fix the DB in function of any SQL statement.
So in summary, it performs a compare DB structure between current DB and 4 alternative sources (Joomla SQL updates, extensions Install SQL scripts, Manifest SQL queries, Other DB structure)
Put our code in Joomla would assume that you have JMS inside Joomla and this is not the case.

For our customers, this is not a problem.
This is just a problem for the other users and this is the reason why we reported that with a basic solution to allow them restore the flexibility that they had in J2.5.

The first time that we presented the "maintenance" menu in JMS was at the Joomla Word Conference 2012 at San Jose with also the Legacy mode for Joomla.
In our maintenance menu and Fix database, we also have an algorithm to help providing a legacy mode on joomla using the same DB.
We also briefly presented this solution present in JMS at the Joomla day Paris 2014.
If you want to review our presentation that we recorded, it is present at the www.ajmool2win.com.

For the current commercial legacy mode for joomla, we preferred the "bridge legacy mode" that contains a fare more sophisticate MySQL parser than the one present in JMS maintenance menu, we used an artificial intelligence technology with Non procedural MySQL processor to perform the alter table and fix database on multiple websites. In other words, we are able to parse and compare multiple websites DB structure and other sources of information in one pass to generate all the alter table required across multiple websites.
This technology is available to our "ajmool2win" customers.

avatar wilsonge
wilsonge - comment - 2 Mar 2015
avatar zero-24
zero-24 - comment - 2 Mar 2015

@jms2win just comment on CS issis. That is why travis fails: https://travis-ci.org/joomla/joomla-cms/jobs/52728258

avatar zero-24
zero-24 - comment - 2 Mar 2015

@jms2win can you have a look into the other both inline comments? :smile:

avatar jms2win
jms2win - comment - 3 Mar 2015

@zero-24 if you are OK to implement the fix correctly, I will close this PR to let you implement it correctly.
Personnally, I don't understand how we have to write that correctly. The PHP is valid and this "travis" report errors that I don't understand.
So I stop trying to write Pull Request code and let other people implement that.

avatar jms2win jms2win - change - 3 Mar 2015
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2015-03-03 10:43:57
avatar jms2win jms2win - close - 3 Mar 2015
avatar jms2win jms2win - close - 3 Mar 2015
avatar wilsonge
wilsonge - comment - 3 Mar 2015

Duuuude don't give up that easy! jms2win#1 I've PR'd a fix into your branch. Merge that and you're good to go here :)

avatar jms2win
jms2win - comment - 3 Mar 2015

Sorry but I already tried several times and this is always the same result and I have PR pending for more than 1 year.
If you would like to make the things easy for styling, why don't you use (or write) a PHP pre-process reformatter to have the style that is required.
Or give a page somewhere that would produce the correct styling reformating like for example the http://beta.phpformatter.com/

avatar wilsonge
wilsonge - comment - 3 Mar 2015

There are autoformatters for all the main IDE's https://github.com/joomla/coding-standards/tree/master/IDE

avatar jms2win
jms2win - comment - 3 Mar 2015

I am not using eclipse for the coding.
I am using the GitHub editor in this website to edit (write) the update

avatar jms2win jms2win - change - 3 Mar 2015
Status Closed New
avatar jms2win jms2win - reopen - 3 Mar 2015
avatar jms2win jms2win - reopen - 3 Mar 2015
avatar jms2win
jms2win - comment - 3 Mar 2015

I tried to re-open to edit it again but it reject.
Sorry but I stop now and let somebody else continue to fix this issue.

avatar jms2win jms2win - change - 3 Mar 2015
Status New Closed
Closed_Date 2015-03-03 10:43:57 2015-03-03 12:00:58
avatar jms2win jms2win - close - 3 Mar 2015
avatar jms2win jms2win - close - 3 Mar 2015
avatar zero-24 zero-24 - reference | - 3 Mar 15
avatar zero-24
zero-24 - comment - 3 Mar 2015

done here @jms2win #6275 Can you add your test there?

avatar jms2win
jms2win - comment - 3 Mar 2015

The test consists in using the Migration method C described in
https://docs.joomla.org/J2.5:Upgrading_from_an_existing_version#Method_C_-_Manual_Upgrade

But it will fails also for other reason that I didn't tried to investigate
Problem of login.

So the procedure consists in remaining logged under J2.5 before unzipping the J3.x update file over the J2.5.28.
See also the video
http://youtu.be/Ip1edGkdY7w
That consisted in the Upgrade from J2.5.28 to J3.2.7 for PHP version reason (<5.3.10)

The objectif of this fix the Migration method C to make it working as supposed it should work.

This method is required when the other method are not working (ie. because fopen_url is closed, upload_max_filesize, ....)

See also the bugtracer
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_id=8103&tracker_item_id=33177

In fact, this is a regression bug introduced in the J3.1.5 when the throw exception were generated instead of a "soft error" (return false).

avatar mbabker
mbabker - comment - 3 Mar 2015

That documentation page needs a major update. As documented, I don't see how any upgrade with that method can work (even in our patch updates, not even considering a 2.5 to 3.x upgrade).

avatar brianteeman
brianteeman - comment - 3 Mar 2015

Thats what I was saying a year ago

On 3 March 2015 at 14:16, Michael Babker notifications@github.com wrote:

That documentation page needs a major update. As documented, I don't see
how any upgrade with that method can work (even in our patch updates, not
even considering a 2.5 to 3.x upgrade).


Reply to this email directly or view it on GitHub
#2839 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Add a Comment

Login with GitHub to post a comment