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

Allow commercial (for a fee) components to use the Joomla! extensions update feature [Tracker 32684] #2508

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
@nikosdion
Contributor

nikosdion commented Nov 13, 2013

Update support for commercial extensions

IMPORTANT: See Joomla! tracker item 32684 at http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32684&start=0

Technical analysis

Status quo

The Joomla! extension update system, up to and including Joomla! 3.2.0, had no support for commercial extensions. This stemmed from the way the updater works. The developer can only specify an update source in his XML manifest which gets saved in the #__update_sites table. The update source is an XML file containing the available version numbers, the platform they support (e.g. Joomla! 3.2 and 2.5) and a download URL. When an update is found, all information except the download URL is saved in the #__updates table.

When a user requests that an extension be updated, through com_installer, the XML update source is parsed again, the download URL is fetched from it and com_installer subsequently downloads and installs the package from that URL.

The big problem with this approach is that the download URL is common for all users. This is perfect for free extensions. It is, however, problematic for commercial extensions. If the developer provides a public URL, common to everyone, for downloading their extension then any user can download and install their commercial software without payment. All a user needs to know is the common download URL. Providing a user-specific download URL means that the developer needs to provide a different XML update source file for each and every of their users. This is impractical and has led to the development of bespoke updaters for commercial extensions.

The major problem with the current situation is the degradation of the user experience. All free extensions' updates are visible in the Control Panel page of Joomla! where the user can easily install them. Commercial extensions' updates are visible only in the respective extensions' interfaces in non-standard ways, essentially being invisible to the user.

Proposal

Considering how important updates are for site security and the benefits of a unified user experience I propose a small scope, backwards compatible change in JUpdater to resolve this situation.

The solution consists of the addition of an extra_query column in the #__update_sites and #__updates table. It contains additional URL query parameters which are appended to the update download URL. Thus the update XML files and the XML manifests of the extensions need not be modified. The only requirement is that the developer populates the column in the #__update_sites table using any mechanism he/she deems appropriate, thereby giving absolute control to the developer over the interface to provide the information necessary for constructing the additional query parameters.

Instructions to developers

Operational concept

The concept of the commercial extensions support hinges on the methodology most of us follow to provide downloads to our commercial extensions, namely the addition of one or more special URL query parameters to our download URLs. The special URL query parameters are used to authenticate the user to our download systems, allowing the download without having the user interactively log in to our sites. For example, if the generic download URL is:

http://www.example.com/com_something.zip

let's say that we need to tack username=sith&key=darth_vader to it in order to authenticate the user. In other words the direct download URL would be:

http://www.example.com/com_something.zip?username=sith&key=darth_vader

The generic download URL will continue to exist in your update XML file, e.g.

<updates>
    <update>
        <name>Something</name>
        <description>Something, something, something component</description>
        <element>com_something</element>
        <type>component</type>
        <version>2.1.0</version>
        <infourl title="Something, something, something component 2.1.0">
            http://www.example.com/something.html
        </infourl>
        <downloads>
            <downloadurl type="full" format="zip">
            http://www.example.com/com_something.zip
            </downloadurl>
        </downloads>
        <tags>
            <tag>stable</tag>
        </tags>
        <maintainer>Example, Inc.</maintainer>
        <maintainerurl>http://www.example.com/</maintainerurl>
        <section>Updates</section>
        <targetplatform name="joomla" version="3.2"/>
        <client>administrator</client>
        <folder/>
    </update>
</updates>

You would, therefore, need to add username=sith&key=darth_vader to the #__update_sites table's extra_query column.

Code example

The code you need to write only needs to do the following:

  • Get the extension_id for your extension (note: this applies to any kind of extension, not just components)
  • Get the parameters to construct the extra URL query parameters string, e.g. from a component's configuration
  • Update the #__update_sites table
  • Flush any existing update records for your extension

Here is a code example for the fictional component com_something, fetching the username and key from the eponymous Options page parameters.

// Get the component information from the #__extensions table
JLoader::import('joomla.application.component.helper');
$component = JComponentHelper::getComponent('com_something');

// Fetch the configuration options and construct the extra URL query
$username = $component->params->get('username', '');
$key = $component->params->get('key', '');

$extra_query = '';

if (!empty($username) && !empty($key))
{
    // Important: you are supposed to use &amp; instead of a straight ampersand
    $extra_query = 'username=' . urlencode($username) . '&amp;key=' . urlencode($key);
}

// Load the update site record, if it exists
$db = JFactory::getDbo();
$query = $db->getQuery(true)
    ->select('update_site_id')
    ->from($db->qn('#__update_sites_extensions'))
    ->where($db->qn('extension_id').' = '.$db->q($component->id));
$db->setQuery($query);
$updateSite = $db->loadResult();

if ($updateSite)
{
    // Update the update site record
    $query = $db->getQuery(true)
        ->update($db->qn('#__update_sites_extensions'))
        ->set(array(
            'extra_query'           => $extra_query,
            'enabled'               => 1,
            'last_check_timestamp'  => 0,
        ))
        ->where($db->qn('extension_id').' = '.$db->q($component->id));
    $db->setQuery($query);
    $db->execute();

    // Delete any existing updates (essentially flushes the updates cache for this update site)
    $query = $db->getQuery(true)
        ->delete($db->qn('#__updates'))
        ->where($db->qn('update_site_id').' = '.$db->q($updateSite->update_site_id));
    $db->setQuery($query);
    $db->execute();
}

This code is relatively naïve (it doesn't check the existing update site) and is only meant as an example, not the definitive implementation.

nikosdion added some commits Nov 12, 2013

Merge branch 'refs/heads/projects-master' into feature-commercial-upd…
…ates

Conflicts:
	administrator/components/com_admin/sql/updates/mysql/3.2.1.sql
	administrator/components/com_admin/sql/updates/postgresql/3.2.1.sql
	administrator/components/com_admin/sql/updates/sqlazure/3.2.1.sql
@wilsonge

This comment has been minimized.

Show comment
Hide comment
@wilsonge

wilsonge Nov 13, 2013

Contributor

At uni so don't have time to test atm. But in 1 word having glanced through this docs: AWESOME :D

Contributor

wilsonge commented Nov 13, 2013

At uni so don't have time to test atm. But in 1 word having glanced through this docs: AWESOME :D

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 13, 2013

Contributor

Thank you, George!

Contributor

nikosdion commented Nov 13, 2013

Thank you, George!

@fastnetwebdesign

This comment has been minimized.

Show comment
Hide comment
@fastnetwebdesign

fastnetwebdesign Nov 13, 2013

Awesome... this is one step closer to something I need to create.

Brilliant... nice work Nik ;-)

fastnetwebdesign commented Nov 13, 2013

Awesome... this is one step closer to something I need to create.

Brilliant... nice work Nik ;-)

@mark-up

This comment has been minimized.

Show comment
Hide comment
@mark-up

mark-up Nov 13, 2013

Excellent PR.

mark-up commented Nov 13, 2013

Excellent PR.

@mbabker mbabker referenced this pull request Nov 13, 2013

Closed

Validation issues #176

@speedy111

This comment has been minimized.

Show comment
Hide comment
@speedy111

speedy111 Nov 13, 2013

Contributor

Well done Nicholas 👍

I'm trying to test it, but I'm having some trouble with updating. Detecting the update is ok. I'm using ARS and the same update URL as I used for Akeeba Live Update. Note to other testers: don't forget to check the format of the URL, it should be XML (not ini...). In stead of username=sith&key=darth_vader I used dlid=YOURDOWNLOADID. That should work, shouldn't it?

Errors:

Warning
Update path does not exist

Message
Error updating COM_INSTALLER_TYPE_TYPE_.
So the type is likely undefined. type of #__update_sites contains extension.

I'll give a look later on, first I'll need a break. The problem is likely PEBKAC ;)

Contributor

speedy111 commented Nov 13, 2013

Well done Nicholas 👍

I'm trying to test it, but I'm having some trouble with updating. Detecting the update is ok. I'm using ARS and the same update URL as I used for Akeeba Live Update. Note to other testers: don't forget to check the format of the URL, it should be XML (not ini...). In stead of username=sith&key=darth_vader I used dlid=YOURDOWNLOADID. That should work, shouldn't it?

Errors:

Warning
Update path does not exist

Message
Error updating COM_INSTALLER_TYPE_TYPE_.
So the type is likely undefined. type of #__update_sites contains extension.

I'll give a look later on, first I'll need a break. The problem is likely PEBKAC ;)

@spignataro

This comment has been minimized.

Show comment
Hide comment
@spignataro

spignataro Nov 13, 2013

Contributor

I +1 this pull request.

Contributor

spignataro commented Nov 13, 2013

I +1 this pull request.

@beat

This comment has been minimized.

Show comment
Hide comment
@beat

beat Nov 13, 2013

Contributor

Nice and simple proposal, Nicholas 👍

The loop would be complete if there is an easy way to automatically populate the extra_query database entry from e.g. an appended "&extra_query=[url-encoded-query-params]" argument in the install-from-url URL (which is also used for the Install-from-Web installation).

Contributor

beat commented Nov 13, 2013

Nice and simple proposal, Nicholas 👍

The loop would be complete if there is an easy way to automatically populate the extra_query database entry from e.g. an appended "&extra_query=[url-encoded-query-params]" argument in the install-from-url URL (which is also used for the Install-from-Web installation).

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Nov 13, 2013

Contributor

@beat If I understand the PR correctly, "Install from web" will work with this out of the box since the extra query will be set by the installation script during installation of the extension or by registering/activate it somehow afterwards. So no need to pass the extra query as URL parameters.

Contributor

Bakual commented Nov 13, 2013

@beat If I understand the PR correctly, "Install from web" will work with this out of the box since the extra query will be set by the installation script during installation of the extension or by registering/activate it somehow afterwards. So no need to pass the extra query as URL parameters.

@beat

This comment has been minimized.

Show comment
Hide comment
@beat

beat Nov 13, 2013

Contributor

@Bakual No, that's just a proposed complement for discussion (if that makes sense, it would be a different PR as Joomla committers love small PRs) ;-)

Contributor

beat commented Nov 13, 2013

@Bakual No, that's just a proposed complement for discussion (if that makes sense, it would be a different PR as Joomla committers love small PRs) ;-)

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 13, 2013

Contributor

@beat It's not a matter of keeping the PRs short because this is an arbitrary rule. It's an engineer understanding exactly which problem they set up to solve, solve only that problem and making sure the solution actually works. This is a fundamental concept in development. It is why we have all moved from Subversion to Git: Subversion favours massive commits, with innevitable scope creep, and penalises feature branches due to its merging woes. Git allows you to make small, strict scope commits and makes strict scope feature branches a breeze.

Now, back to your feature request. I already thought about that before submitting this proposal. It's the job of the installed extension to somehow determine the contents of the extra_query used for updates and nothing but updates. It is architecturally incorrect to use the update feature for installing extensions. For all you know the update package may depend on the extension being already installed. It's not a forbidden practice!

The architecturally correct solution is to allow Install from Web to automatically collect the information required by the extension to subsequently make use of this new feature I wrote to provide updates without the user requiring to enter his/her authentication information again. Here's what I mean. When it comes to the one-off installation URL, the necessary query parameters are already provided by the developer's site. Most likely these can be used to determine the missing bits required to construct the extra_query for updates (e.g. for software distributed using ARS, like mine and Jurian's, it's the Download ID). Even if it's a one-off ID, the extension might be able to submit it to the developer's site and retrieve the information required to construct the extra_query (we need to make sure that JED rules allow this). Your code, the "Install from Web" feature, should cache that installation URL somewhere predictable for the extension to find after it's installed. Therefore you are missing a feature. If you can add it we have a complete and architecturally sound solution.

Contributor

nikosdion commented Nov 13, 2013

@beat It's not a matter of keeping the PRs short because this is an arbitrary rule. It's an engineer understanding exactly which problem they set up to solve, solve only that problem and making sure the solution actually works. This is a fundamental concept in development. It is why we have all moved from Subversion to Git: Subversion favours massive commits, with innevitable scope creep, and penalises feature branches due to its merging woes. Git allows you to make small, strict scope commits and makes strict scope feature branches a breeze.

Now, back to your feature request. I already thought about that before submitting this proposal. It's the job of the installed extension to somehow determine the contents of the extra_query used for updates and nothing but updates. It is architecturally incorrect to use the update feature for installing extensions. For all you know the update package may depend on the extension being already installed. It's not a forbidden practice!

The architecturally correct solution is to allow Install from Web to automatically collect the information required by the extension to subsequently make use of this new feature I wrote to provide updates without the user requiring to enter his/her authentication information again. Here's what I mean. When it comes to the one-off installation URL, the necessary query parameters are already provided by the developer's site. Most likely these can be used to determine the missing bits required to construct the extra_query for updates (e.g. for software distributed using ARS, like mine and Jurian's, it's the Download ID). Even if it's a one-off ID, the extension might be able to submit it to the developer's site and retrieve the information required to construct the extra_query (we need to make sure that JED rules allow this). Your code, the "Install from Web" feature, should cache that installation URL somewhere predictable for the extension to find after it's installed. Therefore you are missing a feature. If you can add it we have a complete and architecturally sound solution.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 13, 2013

Contributor

@jurianeven It's neither a bug in the new feature nor a PEBKAC. It's a long standing bug in Joomla!. JInstaller will recognise the package type by its extension. The download URL generated by ARS looks like http://dev3.local.web/index.php/component/ars/?view=download&id=12&dlid=&dummy=my.zip By setting extra query to dlid=1234567890abcdef1234567890abcdef the effective download URL becomes http://dev3.local.web/index.php/component/ars/?view=download&id=12&dlid=&dummy=my.zip&dlid=1234567890abcdef1234567890abcdef and this makes Joomla! not recognise it as a valid package type. If, however, extra_query is set to dlid=1234567890abcdef1234567890abcdef&jcompat=my.zip then Joomla! recognises it as a ZIP package and installs it. Mind blown? Yeah, same here.

I will do a separate PR to fix JInstaller. As an added bonus it will make the Install from Web feature more reliable. Right now if the installation URL does not end with .zip, .tar, .tar.gz (and possibly .tgz, I have to look at the code again) the package cannot be installed. This has been the case at least since Joomla! 1.5. It's high time we fixed it!

Contributor

nikosdion commented Nov 13, 2013

@jurianeven It's neither a bug in the new feature nor a PEBKAC. It's a long standing bug in Joomla!. JInstaller will recognise the package type by its extension. The download URL generated by ARS looks like http://dev3.local.web/index.php/component/ars/?view=download&id=12&dlid=&dummy=my.zip By setting extra query to dlid=1234567890abcdef1234567890abcdef the effective download URL becomes http://dev3.local.web/index.php/component/ars/?view=download&id=12&dlid=&dummy=my.zip&dlid=1234567890abcdef1234567890abcdef and this makes Joomla! not recognise it as a valid package type. If, however, extra_query is set to dlid=1234567890abcdef1234567890abcdef&jcompat=my.zip then Joomla! recognises it as a ZIP package and installs it. Mind blown? Yeah, same here.

I will do a separate PR to fix JInstaller. As an added bonus it will make the Install from Web feature more reliable. Right now if the installation URL does not end with .zip, .tar, .tar.gz (and possibly .tgz, I have to look at the code again) the package cannot be installed. This has been the case at least since Joomla! 1.5. It's high time we fixed it!

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Nov 14, 2013

Contributor

@nikosdion A bit offopic but is this bug still existent in Joomla 3.2? Because I can install from the following URL (generated by ARS) just fine
http://www.sermonspeaker.net/download/sermonspeaker-component/sermonspeaker-component-5-1-0/pkg_sermonspeaker510-zip.raw
Or is this only an issue in the updater?

Contributor

Bakual commented Nov 14, 2013

@nikosdion A bit offopic but is this bug still existent in Joomla 3.2? Because I can install from the following URL (generated by ARS) just fine
http://www.sermonspeaker.net/download/sermonspeaker-component/sermonspeaker-component-5-1-0/pkg_sermonspeaker510-zip.raw
Or is this only an issue in the updater?

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 14, 2013

Contributor

@Bakual I will have to debug the code to answer that. Last time I checked it was with Joomla! 3.1.

Contributor

nikosdion commented Nov 14, 2013

@Bakual I will have to debug the code to answer that. Last time I checked it was with Joomla! 3.1.

@beat

This comment has been minimized.

Show comment
Hide comment
@beat

beat Nov 14, 2013

Contributor

@nikosdion :

  1. Yes, one PR per issue to solve is good practice, no doubt, now it all depends how fine-grained you divide issues. ;-) This PR is really an atomic grain, and does its own job fine.
  2. Install-from-web is not using the update installer. In Joomla 3.2.0, You can now pass an XML file url to the install by url field, or using the install from web too. You do not have to use your update.xml file. You can use a different one. But if your update.xml file allows to also install (which is usually the case), then you can use the same. Architecturaly this is perfectly ok.
  3. If we keep populating that field under the exclusive responsibility of the extension (e.g. post-install script) itself, then the information of the URL used to install (if installed by URL or with install-from-web) is already available to it in the JInput, and nothing more has to be done imho on a coding side.
  4. JInstaller checks for .xml right now. It could be improved to also check for xml mime types in the http reply to avoid the trick with the mind-blowing extra parameter. PR welcome.

So again to keep my review clear: 👍 from me for this PR.

Contributor

beat commented Nov 14, 2013

@nikosdion :

  1. Yes, one PR per issue to solve is good practice, no doubt, now it all depends how fine-grained you divide issues. ;-) This PR is really an atomic grain, and does its own job fine.
  2. Install-from-web is not using the update installer. In Joomla 3.2.0, You can now pass an XML file url to the install by url field, or using the install from web too. You do not have to use your update.xml file. You can use a different one. But if your update.xml file allows to also install (which is usually the case), then you can use the same. Architecturaly this is perfectly ok.
  3. If we keep populating that field under the exclusive responsibility of the extension (e.g. post-install script) itself, then the information of the URL used to install (if installed by URL or with install-from-web) is already available to it in the JInput, and nothing more has to be done imho on a coding side.
  4. JInstaller checks for .xml right now. It could be improved to also check for xml mime types in the http reply to avoid the trick with the mind-blowing extra parameter. PR welcome.

So again to keep my review clear: 👍 from me for this PR.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 14, 2013

Contributor

Re 1: The smallest possible group of changes that does one job. That's my definition. I come from a Mechanical Engineering background. As a Mech Eng if I have to redesign a gearbox and the smallest design change I can do is change the first gear assembly (drive axle's first gear section and the actual cogwheel of the first gear fitted in the output axle) that's my proposed change. It's not the entire gearbox in one go. The idea is to isolate changes so that you can track down and fix issues more efficiently. As my professor told me ten years ago, engineering a complex machine is not much different than engineering complex software. He was right. The break to smaller steps, solve and assemble approach works wonders ;)

Re 2: No, it's not architecturally OK. As I said, there is nothing preventing a developer from providing partial packages on update (think about how Joomla! provides, say, 2.5.14 to 2.5.15 update packages). If you force update XML files to only provision complete, self-sufficient, installable packages it's a bug (technically: a regression). I'd rather avoid going there. I am sure that someone out there must be doing that partial update thing. Update provisioning is bigger than just JED-listed extensions. I have at least two clients that I know of using ARS to do that.

Re 3: The URL will be in JInput when you first access com_installer. After com_installer finishes (goes into the actual post installation step) the URL is no longer in JInput. Should we store it in the session and let extensions' post-installation scripts use it? That would be an acceptable solution from an architectural point of view.

Re 4: You, are a mind reader! That's one of the two things I want to take a look at today and make PRs. I have stopped whining and started fixing what has been a pain in my bottom for ages. My plan: I want to make sure that both the update XML and the install from URL package use MIME type sniffing to determine the content type and only fall back to extension sniffing if MIME type sniffing is not possible or inconclusive (e.g. serving an update XML file through S3 having forgotten to set the content-type header). I am not yet sure if it's going to be one or two PRs as I have currently no solid view of where the sniffing takes place. I have to RTFC (read the fine code) ;)

Contributor

nikosdion commented Nov 14, 2013

Re 1: The smallest possible group of changes that does one job. That's my definition. I come from a Mechanical Engineering background. As a Mech Eng if I have to redesign a gearbox and the smallest design change I can do is change the first gear assembly (drive axle's first gear section and the actual cogwheel of the first gear fitted in the output axle) that's my proposed change. It's not the entire gearbox in one go. The idea is to isolate changes so that you can track down and fix issues more efficiently. As my professor told me ten years ago, engineering a complex machine is not much different than engineering complex software. He was right. The break to smaller steps, solve and assemble approach works wonders ;)

Re 2: No, it's not architecturally OK. As I said, there is nothing preventing a developer from providing partial packages on update (think about how Joomla! provides, say, 2.5.14 to 2.5.15 update packages). If you force update XML files to only provision complete, self-sufficient, installable packages it's a bug (technically: a regression). I'd rather avoid going there. I am sure that someone out there must be doing that partial update thing. Update provisioning is bigger than just JED-listed extensions. I have at least two clients that I know of using ARS to do that.

Re 3: The URL will be in JInput when you first access com_installer. After com_installer finishes (goes into the actual post installation step) the URL is no longer in JInput. Should we store it in the session and let extensions' post-installation scripts use it? That would be an acceptable solution from an architectural point of view.

Re 4: You, are a mind reader! That's one of the two things I want to take a look at today and make PRs. I have stopped whining and started fixing what has been a pain in my bottom for ages. My plan: I want to make sure that both the update XML and the install from URL package use MIME type sniffing to determine the content type and only fall back to extension sniffing if MIME type sniffing is not possible or inconclusive (e.g. serving an update XML file through S3 having forgotten to set the content-type header). I am not yet sure if it's going to be one or two PRs as I have currently no solid view of where the sniffing takes place. I have to RTFC (read the fine code) ;)

@beat

This comment has been minimized.

Show comment
Hide comment
@beat

beat Nov 14, 2013

Contributor

Re 1: Thank you, Professore Nicholas, we're in sync. :-D Looks like lately, it's trendy to advertise the university degrees in Joomlasphere. I'm not going to follow that trend, even I easly could show off my university, research and business pedigree.

Re 2: Architecturally no issues! Take your breath and read again fully my reply above. You missed a few "not" in the sentences. You can provide a different XML file, specific for installation. If your customers use an incomplete file or misuse their update xmls, it's their problem, not mine, nor an architectural issue.

Re 3: Postinstall script runs in the same page, so no need for session caching imho.

Re 4: Not a mind reader, just logical thinking.

  1. (Dear Github, not "1.", but "5.") While at reading installer code, you will notice new events that I added for the install-from-web feature and with the install-from-web feature that you can benefit from in AkeebaBackup Pro for avoiding to have your own display of Joomla Installer, which doesn't (or didn't?) have Install-from-web tab included, and additionally be able to handle your recovery-points for all installations (including install-from-web and any components-own installs) seamlessly. ;-) Architecturally sure better than overriding the joomla installer alltogether ;-)

So again to keep my review clear: 👍 from me for this PR.

Contributor

beat commented Nov 14, 2013

Re 1: Thank you, Professore Nicholas, we're in sync. :-D Looks like lately, it's trendy to advertise the university degrees in Joomlasphere. I'm not going to follow that trend, even I easly could show off my university, research and business pedigree.

Re 2: Architecturally no issues! Take your breath and read again fully my reply above. You missed a few "not" in the sentences. You can provide a different XML file, specific for installation. If your customers use an incomplete file or misuse their update xmls, it's their problem, not mine, nor an architectural issue.

Re 3: Postinstall script runs in the same page, so no need for session caching imho.

Re 4: Not a mind reader, just logical thinking.

  1. (Dear Github, not "1.", but "5.") While at reading installer code, you will notice new events that I added for the install-from-web feature and with the install-from-web feature that you can benefit from in AkeebaBackup Pro for avoiding to have your own display of Joomla Installer, which doesn't (or didn't?) have Install-from-web tab included, and additionally be able to handle your recovery-points for all installations (including install-from-web and any components-own installs) seamlessly. ;-) Architecturally sure better than overriding the joomla installer alltogether ;-)

So again to keep my review clear: 👍 from me for this PR.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 14, 2013

Contributor

Re 1: Passive aggressive much? I was trying to be polite and explain my definition of small, per your request. Take a chill pill :)

Re 2: As I said, I don't mind using a different file. I mind having to use the same file for both updates and installation. As you said, using a different or the same file is possible right now and I'm cool with that. I am against having the user decide which installation source to use. It's something that only the developer can decide within any degree of accuracy. Is it the user's problem that they used a bad option exposed by Joomla! developers, or is it the Joomla! developers' fault for exposing a bad option? I say it's the Joomla! developers' fault, making it a bad architecture decision. You said that right now the developer can choose to serve the same XML file and that is perfectly fine! Ain't broken, don't fix it.

Re 3: Um, I actually consider the current architecture in com_installer a problem, as it's trying to download and install the extension in one go. On slow shared hosting this is a major issue and I'd like to fix that. It's further down my to-do list. Corollary: you won't have the URL in JInput. That's a solution I've tried with great success in my Live Update, that's why I want to fix that semi-broken architecture. Sorry, I forgot I never had that discussion with you. Is it more clear why I propose saving the URL in the session now instead of relying to JInput?

Re 5: The new features are not even necessary as far as I can see, but it's anyway item number 4 on my to-do list. The current feature dates back from the Joomla! 1.5 days and was scheduled to be refactored a year ago. The exact circumstances that led to this feature stagnating are beyond the scope of this PR so I'll spare you the gory details. We can talk about it later today on Skype if you wish.

Contributor

nikosdion commented Nov 14, 2013

Re 1: Passive aggressive much? I was trying to be polite and explain my definition of small, per your request. Take a chill pill :)

Re 2: As I said, I don't mind using a different file. I mind having to use the same file for both updates and installation. As you said, using a different or the same file is possible right now and I'm cool with that. I am against having the user decide which installation source to use. It's something that only the developer can decide within any degree of accuracy. Is it the user's problem that they used a bad option exposed by Joomla! developers, or is it the Joomla! developers' fault for exposing a bad option? I say it's the Joomla! developers' fault, making it a bad architecture decision. You said that right now the developer can choose to serve the same XML file and that is perfectly fine! Ain't broken, don't fix it.

Re 3: Um, I actually consider the current architecture in com_installer a problem, as it's trying to download and install the extension in one go. On slow shared hosting this is a major issue and I'd like to fix that. It's further down my to-do list. Corollary: you won't have the URL in JInput. That's a solution I've tried with great success in my Live Update, that's why I want to fix that semi-broken architecture. Sorry, I forgot I never had that discussion with you. Is it more clear why I propose saving the URL in the session now instead of relying to JInput?

Re 5: The new features are not even necessary as far as I can see, but it's anyway item number 4 on my to-do list. The current feature dates back from the Joomla! 1.5 days and was scheduled to be refactored a year ago. The exact circumstances that led to this feature stagnating are beyond the scope of this PR so I'll spare you the gory details. We can talk about it later today on Skype if you wish.

@beat

This comment has been minimized.

Show comment
Hide comment
@beat

beat Nov 14, 2013

Contributor

Re 1: Thanks for clarifying 👍 (no punching intended) :-D

Re 2: Ok, then not "No, it's not architecturally OK" as I understood above. So it's architecturally OKay 👍

Re 3: Nw, understand now: Right now in 3.2.0, it is available in JInput, as it is in same page, and if you do a PR to change it for a 2-step-process, then yes, the corresponding input would be lost and should be preserved. I guess that would be the responsibility of the new PR to preserve it in JInput using session for B/C (and, maybe to provide a new clean API for it), if I understand it right ?

Re 5: No worries. Fully understand. The new installer-type plugin events now allow you to do what you planed (even with current installer). Me too didn't want to rewrite the installer as part of web-installer improvements for exact same change-control granularity as we agree in "1."). 👍

So again to keep my review clear: 👍 from me for this PR.

Contributor

beat commented Nov 14, 2013

Re 1: Thanks for clarifying 👍 (no punching intended) :-D

Re 2: Ok, then not "No, it's not architecturally OK" as I understood above. So it's architecturally OKay 👍

Re 3: Nw, understand now: Right now in 3.2.0, it is available in JInput, as it is in same page, and if you do a PR to change it for a 2-step-process, then yes, the corresponding input would be lost and should be preserved. I guess that would be the responsibility of the new PR to preserve it in JInput using session for B/C (and, maybe to provide a new clean API for it), if I understand it right ?

Re 5: No worries. Fully understand. The new installer-type plugin events now allow you to do what you planed (even with current installer). Me too didn't want to rewrite the installer as part of web-installer improvements for exact same change-control granularity as we agree in "1."). 👍

So again to keep my review clear: 👍 from me for this PR.

@speedy111

This comment has been minimized.

Show comment
Hide comment
@speedy111

speedy111 Nov 18, 2013

Contributor

Thanks Nicholas, appending &jcompat=my.zip fixed it. I've marked the test as successfull on the tracker. I only had my extensions' postinstall message rendered in a <div class="alert alert-info">, making the whole output blue. But I think that's either just the way the installer renders it or I have to adapt my installer script.

Contributor

speedy111 commented Nov 18, 2013

Thanks Nicholas, appending &jcompat=my.zip fixed it. I've marked the test as successfull on the tracker. I only had my extensions' postinstall message rendered in a <div class="alert alert-info">, making the whole output blue. But I think that's either just the way the installer renders it or I have to adapt my installer script.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 18, 2013

Contributor

Jurian,

The postinstall message renders in a clear div. The alert-info class has to come from something in your custom component installation script. Just to be clear, I did not touch the code that renders those messages.

Contributor

nikosdion commented Nov 18, 2013

Jurian,

The postinstall message renders in a clear div. The alert-info class has to come from something in your custom component installation script. Just to be clear, I did not touch the code that renders those messages.

@speedy111

This comment has been minimized.

Show comment
Hide comment
@speedy111

speedy111 Nov 18, 2013

Contributor

I also didn't suspect you've touched it. I see it's definitely coming from com_installer, it wraps the installer code.

See:
joomla-cms/administrator/components/com_installer/views/update/tmpl/default.php (line 30)

   <?php if ($this->showMessage) : ?>
        <div class="alert alert-info">
            <a class="close" data-dismiss="alert" href="#">×</a>
            <?php echo $this->loadTemplate('message'); ?>
        </div>
    <?php endif; ?>

It works when I use this in stead:

   <?php if ($this->showMessage) : ?>
        <div>
            <?php echo $this->loadTemplate('message'); ?>
        </div>
    <?php endif; ?>

Again, this is unrelated to this patch, but it's an issue though.

Contributor

speedy111 commented Nov 18, 2013

I also didn't suspect you've touched it. I see it's definitely coming from com_installer, it wraps the installer code.

See:
joomla-cms/administrator/components/com_installer/views/update/tmpl/default.php (line 30)

   <?php if ($this->showMessage) : ?>
        <div class="alert alert-info">
            <a class="close" data-dismiss="alert" href="#">×</a>
            <?php echo $this->loadTemplate('message'); ?>
        </div>
    <?php endif; ?>

It works when I use this in stead:

   <?php if ($this->showMessage) : ?>
        <div>
            <?php echo $this->loadTemplate('message'); ?>
        </div>
    <?php endif; ?>

Again, this is unrelated to this patch, but it's an issue though.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 18, 2013

Contributor

I wonder when that was added. I was testing with Joomla! 3.2 yesterday and didn't see that :s

Contributor

nikosdion commented Nov 18, 2013

I wonder when that was added. I was testing with Joomla! 3.2 yesterday and didn't see that :s

@speedy111

This comment has been minimized.

Show comment
Hide comment
@speedy111

speedy111 Nov 18, 2013

Contributor

I had one update and after installing it I got the message: "There are no updates available at the moment. Please check again later.". Maybe it's related, but I don't see it in the code.

Contributor

speedy111 commented Nov 18, 2013

I had one update and after installing it I got the message: "There are no updates available at the moment. Please check again later.". Maybe it's related, but I don't see it in the code.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 18, 2013

Contributor

That sounds about right, you installed the update, there is no available update.

Contributor

nikosdion commented Nov 18, 2013

That sounds about right, you installed the update, there is no available update.

@wilsonge

This comment has been minimized.

Show comment
Hide comment
@wilsonge

wilsonge Nov 18, 2013

Contributor

Just traced it back and it was for J3.0.0 when we bootstrapped the administrator (e11117d#diff-c522b589c32e0f38c81e2151a3a64d2cL20)

Contributor

wilsonge commented Nov 18, 2013

Just traced it back and it was for J3.0.0 when we bootstrapped the administrator (e11117d#diff-c522b589c32e0f38c81e2151a3a64d2cL20)

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Nov 18, 2013

Contributor

Thanks @wilsonge I probably never spotted it as I'm wrapping my messages inside a div to give them full width (required for the tables I use).

Contributor

nikosdion commented Nov 18, 2013

Thanks @wilsonge I probably never spotted it as I'm wrapping my messages inside a div to give them full width (required for the tables I use).

@speedy111

This comment has been minimized.

Show comment
Hide comment
@speedy111

speedy111 Nov 18, 2013

Contributor

(@nicholas Haha yes, for sure. But perhaps that triggers the blue alert box. I don't see this in that code though.)

@wilsonge I see, there this issue seems to be introduced.

Contributor

speedy111 commented Nov 18, 2013

(@nicholas Haha yes, for sure. But perhaps that triggers the blue alert box. I don't see this in that code though.)

@wilsonge I see, there this issue seems to be introduced.

@speedy111

This comment has been minimized.

Show comment
Hide comment
@speedy111

speedy111 Nov 18, 2013

Contributor

I created a tracker with fix, just like it was before that commit:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32742&start=0

Contributor

speedy111 commented Nov 18, 2013

I created a tracker with fix, just like it was before that commit:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32742&start=0

@rbismut

This comment has been minimized.

Show comment
Hide comment
@rbismut

rbismut Feb 21, 2017

where can i run such a code (saving did) in case of a module?

rbismut commented Feb 21, 2017

where can i run such a code (saving did) in case of a module?

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Feb 21, 2017

Contributor

@rbismut Please don't post on closed PR/issues.

Contributor

Bakual commented Feb 21, 2017

@rbismut Please don't post on closed PR/issues.

@mridulcs2012

This comment has been minimized.

Show comment
Hide comment
@mridulcs2012

mridulcs2012 Apr 24, 2017

As i see in the code you are updating #__update_sites_extensions tables instead #__update_sites table, is this typing mistake?

mridulcs2012 commented Apr 24, 2017

As i see in the code you are updating #__update_sites_extensions tables instead #__update_sites table, is this typing mistake?

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Apr 24, 2017

Contributor

This is a very old tracker item. Please post a new issue if you have found a bug and not comment on closed issues. Thanks.

Contributor

zero-24 commented Apr 24, 2017

This is a very old tracker item. Please post a new issue if you have found a bug and not comment on closed issues. Thanks.

@joomla joomla locked and limited conversation to collaborators Apr 24, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.