Skip to content
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

Fix problem load ExtensionPackage from cache #12359

Closed
wants to merge 1 commit into from
Closed

Fix problem load ExtensionPackage from cache #12359

wants to merge 1 commit into from

Conversation

Fi1osof
Copy link
Contributor

@Fi1osof Fi1osof commented Feb 11, 2015

What does it do ?

Fix problem load ExtensionPackage from cache

Why is it needed ?

Because we have errors: Problem getting service basket, instance of class Basket, from path

@Mark-H Mark-H added this to the v2.3.6-pl milestone Jul 12, 2015
@Mark-H
Copy link
Collaborator

Mark-H commented Jul 12, 2015

I've seen this error quite a bit with MoreGallery (which loads an extension package for the custom resource), but I can't seem to recreate it on a clean install.. maybe the error has something to do with upgrades? Your commit mentions the cache, maybe you remember the steps needed to trigger the issue? If you don't remember, I don't blame you, this PR has been here for a while..

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Jul 12, 2015

@Mark-H Maybe fixed in last version. After 2.3.3 publishing many people reports this. http://modxclub.ru/topics/oshibka-podgruzki-klassov-1566.html

@Mark-H
Copy link
Collaborator

Mark-H commented Jul 12, 2015

Hmmm so 2.3.3... maybe related to this ceac117 which was pull #11927

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Jul 13, 2015

Yes, it seems.

@Fi1osof Fi1osof closed this Jul 13, 2015
@Fi1osof
Copy link
Contributor Author

Fi1osof commented Jul 27, 2015

Problem exists after update from modx2.2 to modx2.3

@Fi1osof Fi1osof reopened this Jul 27, 2015
@theboxer theboxer modified the milestone: v2.3.6-pl Aug 18, 2015
@opengeek opengeek closed this Sep 14, 2015
@theboxer theboxer reopened this Sep 14, 2015
@Fi1osof
Copy link
Contributor Author

Fi1osof commented Oct 25, 2015

I understood. Problem exists when setted serviceName and serviceClass, for example:

{"shopModx":{"serviceName":"shopModx","serviceClass":"shopModx","path":"[[++core_path]]components/shopmodx/model/"}}

Try to install shopModx.

It`s actual for latest version of MODX.

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Feb 5, 2016

@Mark-H, hi!
Is this will merged?
I tired to fix this on every site. http://joxi.ru/ZrJVWD0u1kLgPr
PR created 11 Feb 2015. Almost a year ago.

@christianseel
Copy link
Contributor

I have an extension package specified with serviceName and serviceClass in a project and I don't get those error messages. So maybe there's something wrong with your configuration anywhere @Fi1osof?

This is my example:

    {
        "phpconsole": {
            "serviceName": "phpconsole",
            "serviceClass": "phpconsoleX",
            "path": "[[++core_path]]components/phpconsole/model/"
        }
    }

…and the class file is located at /core/components/phpconsole/model/phpconsole/phpconsolex.class.php

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Feb 5, 2016

@christianseel MODX version?

@Fi1osof Fi1osof closed this Feb 5, 2016
@Fi1osof Fi1osof reopened this Feb 5, 2016
@christianseel
Copy link
Contributor

@Fi1osof 2.4.2-pl

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Feb 5, 2016

@christianseel i use 2.4.2 too.
I tested this bug many times and know this is bug, not my error (and not only i catch this error).
My config {"basket":{"serviceName":"basket","serviceClass":"Basket","path":"[[++core_path]]components/basket/model/"}}

It`s happens only on MODX init. Then OK.

@christianseel
Copy link
Contributor

What do you mean by "MODX init"? Loading frontend with an empty cache?

Could you provide full steps to reproduce the bug? Because for me it looks like #11927 should have fixed those issues. I remember I had similar issues with my phpconsole package, was that was because of incorrect paths and configs and not a bug in MODX.

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Feb 6, 2016

@christianseel @Mark-H i finded mistake.
It`s happends when extension_packages stored in database. http://joxi.ru/8AnXDv9uqKB7Zm
I dont know how they got there. modX::addExtensionPackage() writes sistem settins only, do not create modExtensionPackage objects.

modX::_loadExtensionPackages() get modExtensionPackages, and if got, load them. And those have broken pathes.

@opengeek, why this for?

@christianseel
Copy link
Contributor

Great! Maybe @rtripault can clarify here, since he was also doing #11927 which dealed with extension packages in the database.

@rtripault
Copy link
Contributor

I unfortunately won't be able to provide more details on why the extension packages are stored in a dedicated table (besides it's happening when migrating from Revo 2.2, see https://github.com/modxcms/revolution/blob/2.x/setup/includes/upgrades/common/2.3-extension-packages.php).
My best "bet" (other than those comments) on this decision is the limitation of modSystemSetting.value size (text type) in case there are loads of packages.

In fact, i feel like the current implementation of modExtensionPackage (which is not complete, as @Fi1osof noticed) is a "regression", since we are not able to define the order of loaded packages/services since they are loaded by namespace name (see https://github.com/modxcms/revolution/blob/2.x/core/model/modx/modcachemanager.class.php#L448) while, with some trick (ordering the JSON data stored in the system setting), we could influence that order using the system setting implementation.

Hope that will help a bit anyway

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Feb 9, 2016

@rtripault

since we are not able to define the order of loaded packages/services since they are loaded by namespace name (see https://github.com/modxcms/revolution/blob/2.x/core/model/modx/modcachemanager.class.php#L448) while, with some trick (ordering the JSON data stored in the system setting), we could influence that order using the system setting implementation.

I wrote this too almost 3 years ago http://joxi.ru/DrlaPn9i4nQodm

It`s sad that ExtensionPackages functionality is not centralized

@OptimusCrime
Copy link
Contributor

This PR is over two years old. Can it be closed?

@opengeek opengeek closed this Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants