Code Monkey home page Code Monkey logo

Comments (75)

AmyStephen avatar AmyStephen commented on August 16, 2024

Full support. Already, very encouraged by comments on Twitter, not only from Joomla devs who will certainly welcome this, but maybe even more importantly from some of the hard-core PHP devs whose involvement would be awesome.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

Don, first off, thanks for putting the work in so far. However, understand we must put this through the refiners fire so I'm just going to say stick with it as we field comments. This is also, probably, going to be an iterative process. So, a couple of points.

I have personally committed to maintaining

I get what you are saying but this is an RFC and we should keep the text second or third person. I think it's best that you change split that block of text and create a new section called "Backward Compatibility". Then you propose to introduce the change en masse in 13.1 to be deprecate the compatibility layer in 14.1. Just thinking out loud though, another way to handle this is to say that the compatibility layer will be introduced in 13.1 but will be "frozen" after 13.3. That would mean that any new work must be referenced by namespace; and old classes could referenced by the old class names ad infinitum. The compatibility layer could also be opt in, you have to explicitly turn it on much like you have to explicitly add the legacy tree.

You haven't mentioned anything about how file names work. If there is no change to file and folder structure, this should be mentioned explicitly. If change is required, you also need to map out what the changes are.

I think the experienced Platform contributors will tend to get the impact of change from the examples you've given. However, I don't think the average CMS component developer is going to get it. It's probably worth adding several examples revolving around how this would affect CMS developers IF the CMS decided to adopt 13.1+ at some time in the future. Obviously we can't predict what the CMS would do, but we need to have a go at providing all the information we can so that any downstream user can make an informed decision (it will certainly affect what I do at work this year big time).

I think the dot points under "Benefits of Namespacing" need a sentence or paragraph after each point to further explain what is meant by, for example, "Eliminate ambiguity". What does that mean to an average downstream users that has never heard of namespacing. Remember, we are dealing with developers that kicked up a huge stink over dropping the DS constant - this is going to blow their mind.

Currently we have a system of overrides in the loader. I can create my own JApplicationWeb and have that override the Platform's core class. It's worth explaining how that feature would be supported under name spacing or how that feature has to change.

Obviously the task breakdown is going to be huge for this effort (the unit testing for this is probably going to be daunting, and we may have quite a number of dependencies like absolutely getting rid of lingering CMS code). Hopefully David will have Jira up by then :) In the absence of the Jira, I think it's worth starting the "task list" in the RFC just so we know the full impact of what we are signing up for. I know you are going to shoulder a lot of the work, but you are going to need help :)

Also, once we are happy with the state of the RFC, we are going to need to publish this widely and will probably need to do a voting response survey like we did for the LGPL.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Andrew,

Thanks for the pointers - I'll rework some of the language in the RFC and get this right. It's going to get lengthy. :)

As for a task list, it definitely is greater than just throwing some namespace and use constructs at the tops of files and calling it a day. I initially thought it could be a simple exercise - but I see it is definitely more than that now. No worries - I'm in this for the long haul.

How namespaces are structured and how we import classes between packages and such has a great impact on the flexibility of the platform in the future.

One of the bigger tasks at this point is clearly defining and separating packages to make them independent.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

All good points @eddieajau - as an aside, it's a lot easier to complain about DS because it's easier to understand. Given your observation, I think this effort might just get ignored. ;-)

Don and I were just talking about how one approach to implementing NS can result in flexible overrides using DI, or dependency chains that are very hard to override. https://gist.github.com/4458255/ The approach does indeed make a difference - glad you raised that point.

The other issue to work out is a decision on whether or not (or how) a services container might fit in as it can provide some relief with NSing. Andrew - Do you think it's the right time to decide on this? In conjunction with the NSing? Or, do you think it's better to wait on that? Dealing with JFactory in some way will be impossible to ignore.

This effort will drive a lot of good direction, really glad to see you guys have been working on this.

I'll ping @dukeofgaming this weekend, see if he's around.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

Task I see should be in the mix:

  • 100% code coverage for loader related classes, including the compatibility layer
  • 100% skeleton coverage - by that I mean we should at least have skeleton tests for all classes that includes the fully namespaced classes.
  • All changes are well documented in the Platform Manual.
  • Review and move lingering CMS classes to the legacy tree (JDocument, JPlugin, etc)

Amy, yes, a standing goal should be to design the Platform with dependency injection in mind. However, I don't know what you mean by "services container".

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Edit: Seemless interoperability requires file structure to match namespace casing as well.

Ex: Joomla\Application\Web\Router would map to Joomla/Application/Web/Router.php

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

Bugga. It's worth noting that will be a required change in the file system. That's not a particularly bad one, but one we need to make clear.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

On, and might be better to include a more translatable word instead of "anathema". Fitting, but not common :)

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Ya, I'm not happy about it either - but I think the revitalized interest in the platform is worth the extra effort. I got used to camelCased method names, I can get used to the same for structure.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Good news is - using git mv we can easily retain all file history. That's a plus.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

@eddieajau Good resource is from Fowler

http://martinfowler.com/articles/injection.html#ServiceLocatorVsDependencyInjection

Mainly a question of what you want to do with JFactory, thinking there is plenty already to do, as it is.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

JFactory is a separate discussion in my book. I think it's scope creep for
this discussion.

Regards,
Andrew Eddie
http://learn.theartofjoomla.com - training videos for Joomla developers

On 5 January 2013 14:26, AmyStephen [email protected] wrote:

@eddieajau https://github.com/eddieajau Good resource is from Fowler

http://martinfowler.com/articles/injection.html#ServiceLocatorVsDependencyInjection

Mainly a question of what you want to do with JFactory, thinking there is
plenty already to do, as it is.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1787#issuecomment-11909815.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

The first 27 slides here are relevant to IoC, dependency injection, etc (how I think we could use it) - http://www.slideshare.net/neraath/ioc-with-php-8833643

But, as you said, outside scope.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

So, correct me if I'm wrong, but PSR-0 requires that underscore must be converted to a directory separator, but it doesn't preclude you from having any other rules that you desire. You could, for example, make each character of the class name a folder and you'd still comply with PSR-0, no?

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

I'm not so sure that's true. The idea is that a common class loader be usable for work from various authors/projects.

It might even be worthwhile to use the Classloader that comes with composer (if you don't include your own). That's going to be the best way to verify compliance.

https://github.com/composer/composer/blob/master/src/Composer/Autoload/ClassLoader.php

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Yes and no. They chose to split on _ because that's how PEAR and Zend functioned before native NS capabilities. You would still be 'compliant' to the standard if you did what you said, because the standard fails to mention that in order for the autoloader to work, it needs to specifically exclude basing your directory structure an anything else.

The goal of the RFC is for our code and others code to be able to work with the auto loader they and we are already using. JLoader::loadByNaturalCase is almost there, although I discovered a bug in it last night.

Sent from my iPhone

On Jan 5, 2013, at 3:17 AM, Andrew Eddie [email protected] wrote:

So, correct me if I'm wrong, but PSR-0 requires that underscore must be converted to a directory separator, but it doesn't preclude you from having any other rules that you desire. You could, for example, make each character of the class name a folder and you'd still comply with PSR-0, no?


Reply to this email directly or view it on GitHub.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

Agree with the goal aspect of Don's response. Unfortunately, I don't see ambiguity in the rule. The PSR-0 also states "You can test that you are following these standards by utilizing this sample SplClassLoader implementation which is able to load PHP 5.3 classes." Following that statement is an Autoload function with code.

So, bottom line is, if that code loads our classes, given our NS strategy, we're in compliance. If it doesn't, we're not. I really do think it's as simple as that.

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

We might consider adding unit testing that brings in the shared class loader, if JLoader is maintained. I wonder if it makes sense for the platform to simply use the class loader from Composer? If JLoader is needed for the CMS during the deprecation period, would it be better to move it into the CMS branch? An idea, anyway.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

By inspection of the code, the RFC is not going to be PSR-0 compliant:

https://gist.github.com/221634#file-splclassloader-php-L131

IF that's what the PSR meant - and it probably did. It's not what the PSR
says and this was done before the group implemented the MUST's and SHOULD's.

It just means we aren't going to fully compatible (but we will be fully
compliant to the letter of the law) in terms of people wanting to consume
the Platform with other PSR-0 underscore split code.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

@eddieajau - I can't see an issue with Joomla's compliance. I only know of one class - and that's the numeric class name in the Crypt package. Other than that, I am not seeing any challenge to compliance. Sorry to be so dense but I am not seeing the problem. What problems are you seeing?

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

The whole exercise of name spacing for me is based around 2 things. The ability for platform users to consume PSR-0 packages and for PSR-0 users to be able to seamlessly use platform code.

Sent from my iPhone

On Jan 5, 2013, at 4:00 PM, Andrew Eddie [email protected] wrote:

By inspection of the code, the RFC is not going to be PSR-0 compliant:

https://gist.github.com/221634#file-splclassloader-php-L131

IF that's what the PSR meant - and it probably did. It's not what the PSR
says and this was done before the group implemented the MUST's and SHOULD's.

It just means we aren't going to fully compatible (but we will be fully
compliant to the letter of the law) in terms of people wanting to consume
the Platform with other PSR-0 underscore split code.

Reply to this email directly or view it on GitHub.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

It really is a little frustrating that there are some many PSR-0 class loaders that are pointed too as the "official" standard enforcer. @eddieajau - the one you are pointing to was a very early copy and it doesn't appear to have been updated in a long time. Maybe that's the code problem you are referring too? (I hope)

The one I believe we should be most focused on is the one provided with Composer.
https://github.com/composer/composer/blob/master/src/Composer/Autoload/ClassLoader.php

Again, sorry to repeat, but if there are compliance issues for Joomla, let's identify very clearly why we cannot comply. If it's only a matter of providing time to deprecate old names, that is not a compliance problem. The CMS will have to have a different autoloader for a period of time. That would be expected.

But, if there is something we can't do and there is a valid reason for us doing it a certain way (meaning, not just a preference, needs to be a valid reason) then we should fork the class loader, adapt it, so it works for everyone else and for us, and have Andrew, as a voting member of the PSR group, present our case and request adaptations needed.

But, again, I just don't understand yet what the problem is. What @dongilbert has presented is in compliance, if I understand things, and if it's not, I would appreciate an explanation. I am just not getting seeing the problem.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

OH - I apparently had a brain fart when I read your response earlier Andrew.

I think you are referring to my statement that _'s are anathema and not to be used in our code whatsoever.

The reason I made that statement is that I don't want our code to have classes with _'s in any situation. (I was considering using it earlier to solve the Cipher\3DES problem, but I decided against it.)

Our code will be compliant and able to be consumed by PSR-0 autoloaders / projects.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

Good.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

@amy, the gist I referred to is what is in PSR-0 "standard".

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

See the very last link.

In terms of compliance, PSR-0 does not say definitively that underscores are the only way you can determine how to form the real file system path. Whether that is what is intended or not is neither here nor there, PSR-0 is silent on the matter. On that basis I think we can rightly say we will be able to "comply" to the letter of the law with PSR-0, but that does not necessarily mean we will be compatible with everyone. It's a deficiency in PSR-0. The bigger problem we have is breaking on camel case because some libraries, like Zend, have files like /Foo/Bar/GooCar.php.

As for changing the PSR, I'm happy to take a case to the group but I don't want to own the processing of making "our" case. Amy, maybe you'd like to run with doing the work of making a draft and getting our community to discuss it. If the consensus is that we need to propose a change to PSR-0, I can take it to FIG. If nobody is interested in forming a case for change, we'll just keep on doing our own thing (maybe I need to say something about that on the mailing list).

Just on that point I just want to make it clear I only have time to be a voting member of issues that interest this community. I don't have time to keep you up to date with what they are discussing etc and so on. You can subscribe to the mailing list yourself for that :) And as I've said, not every PSR is going to fit what we do - the PSR's are about normalising the average, not necessarily about being innovative and bleeding edge which is more where I'd like to see the culture of the Platform head.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

The PSR does say that it splits on namespace separator and on underscore. It goes on to say that casing in class name has no effect. So it could be assumed that if casing in class name caused an effect, that it is in fact non-complaint to the standard.

TBH I want name spacing and LGPL so that the great code in the platform can be consumed by others, and so Joomla can be a respected platform once again. If you're set against that, then I drop my case for both items.

Sent from my iPhone

On Jan 5, 2013, at 8:22 PM, Andrew Eddie [email protected] wrote:

@amy, the gist I referred to is what is in PSR-0 "standard".

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

See the very last link.

In terms of compliance, PSR-0 does not say definitively that underscores are the only way you can determine how to form the real file system path. Whether that is what is intended or not is neither here nor there, PSR-0 is silent on the matter. On that basis I think we can rightly say we will be able to "comply" to the letter of the law with PSR-0, but that does not necessarily mean we will be compatible with everyone. It's a deficiency in PSR-0. The bigger problem we have is breaking on camel case because some libraries, like Zend, have files like /Foo/Bar/GooCar.php.

As for changing the PSR, I'm happy to take a case to the group but I don't want to own the processing of making "our" case. Amy, maybe you'd like to run with doing the work of making a draft and getting our community to discuss it. If the consensus is that we need to propose a change to PSR-0, I can take it to FIG. If nobody is interested in forming a case for change, we'll just keep on doing our own thing (maybe I need to say something about that on the mailing list).

Just on that point I just want to make it clear I only have time to be a voting member of issues that interest this community. I don't have time to keep you up to date with what they are discussing etc and so on. You can subscribe to the mailing list yourself for that :) And as I've said, not every PSR is going to fit what we do - the PSR's are about normalising the average, not necessarily about being innovative and bleeding edge which is more where I'd like to see the culture of the Platform head.


Reply to this email directly or view it on GitHub.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

Ok, so I've been a bit confused and that is all clear up now. In summary, what we can do is say:

a) the Platform will conform to PSR-0 in every way
b) BUT, we will enforce some style rules

The style rules will be that we must manually namespace every class to generate the effect of the camel case being a folder/file break.

We generally won't allow underscores in the class names UNLESS the class name would start with a number. In that case, the crypt ciphers would be Cipher_3des, Cipher_Blowfish - a one-in-all-in deal. We could support Cipher3des or CipherBlowfish but, in my opinion, it's better to have those in their own folder.

End result, I'm all good with PSR-0, aside from the work it obviously generates :)

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

YAY

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

@eddieajau This has been confusing to me, as well, I've used a lot of Don's time trying to follow. I am concerned you might still have a slightly different understanding when you say this:

"The style rules will be that we must manually namespace every class to generate the effect of the camel case being a folder/file break."

I guess I'm not clear on what you mean by that. In reality, every class is namespaced, but how you feed the class loader is by registering a namespace and associating it with a folder location. Then, those registered namespaces are used to search for classes.

I believe Don's plan is to register the namespace for each package. In my Autoload, I would have statements like the following for each package I want to use from JPlatform:

require_once VENDOR . '/symfony/Component/ClassLoader/UniversalClassLoader.php';
use symfony\Component\ClassLoader\UniversalClassLoader;

$s = new UniversalClassLoader();

$s->registerNamespace('JPlatform\Crypt', BASE_FOLDER . '/Vendor');
$s->registerNamespace('JPlatform\Database', BASE_FOLDER . '/Vendor');
$s->registerNamespace('JPlatform\Log', BASE_FOLDER . '/Vendor');

and so on.

For the log package, only one namespace would be registered with the class loader.

Taking the entry class as an example:

  1. it would have a single namespace line at the top of the file for the namespace that represents the folder containing the file. It might be a registered namespace, but if the files are in a subfolder for the package, it will be the registered namespace + the name of the subfolder(s). That is true for each and every file -- it must have a namespace line -- that must be the first (non-comment) line - and that just PHP.

Now, this next point I was confused on and I am thinking you might be, as well.

  1. The class name defined within the file will change. Instead of having a class named JLogEntry -- which translates to joomla\log\entry, the class will be named Entry. And that's good, because the file is named entry.php. (Although we might need to uppercase the first letter of those filenames.)

So, the top of the JLogEntry file will look like this:

namespace JPlatform\log;

class Log
{
etc.

If I wanted to use this class in another class, I would instantiate it as:

$class = 'JPlatform\log\Log';
$instance = new $class();

or I could define a use statement on the top of the page (don't recommend doing it this way as overrides become difficult):

namespace in\my\class\folder;
use Jplatform\log\Log;

Then within a method:

$instance = new Log();

Is that your understanding, too? Or, are you thinking the old JLogEntry class name will continue to be used for the class definition? (because that is not the plan - and it was a point of confusion for me until Friday.)

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Amy, the vendor name will be Joomla, not JPlatform. Also, for the time
being, we will only be registering the root namespace of Joomla, not each
package.

Overrides would function much the same. From what I understand, you can
register multiple locations to search, just like we currently do.

One thing I did notice though is that no part of the namespace is trimmed
off when loading via the standard loader. Meaning it would be registered
this way:

$loader->registerNamespace('Joomla', JPATH_LIBRARIES);

And then for the CMS to override classes would have:

$loader->registerNamespace('Joomla', JPATH_LIBRARIES . '/Cms);

Then, for classes for the CMS to override would go in /libraries/Cms/Joomla.

A name spaced JApplicationSite would go in
/libraries/Cms/Joomla/Application/Site.php or it could be renamed to
Cms/Application/Site.php

On Saturday, January 5, 2013, AmyStephen wrote:

@eddieajau https://github.com/eddieajau This has been confusing to me,
as well, I've used a lot of Don's time trying to follow. I am concerned you
might still have a slightly different understanding when you say this:

"The style rules will be that we must manually namespace every class to
generate the effect of the camel case being a folder/file break."

I guess I'm not clear on what you mean by that. In reality, every class is
namespaced, but how you feed the class loader is by registering a namespace
and associating it with a folder location. Then, those registered
namespaces are used to search for classes.

I believe Don's plan is to register the namespace for each package. In my
Autoload, I would have statements like the following for each package I
want to use from JPlatform:

require_once VENDOR .
'/symfony/Component/ClassLoader/UniversalClassLoader.php';
use symfony\Component\ClassLoader\UniversalClassLoader;

$s = new UniversalClassLoader();

$s->registerNamespace('JPlatform\Crypt', BASE_FOLDER . '/Vendor');
$s->registerNamespace('JPlatform\Database', BASE_FOLDER . '/Vendor');
$s->registerNamespace('JPlatform\Log', BASE_FOLDER . '/Vendor');

and so on.

For the log package, only one namespace would be registered with the class
loader.

Taking the entry class as an example:

  1. it would have a single namespace line at the top of the file for
    the namespace that represents the folder containing the file. It might be a
    registered namespace, but if the files are in a subfolder for the package,
    it will be the registered namespace + the name of the subfolder(s). That is
    true for each and every file -- it must have a namespace line -- that must
    be the first (non-comment) line - and that just PHP.

Now, this next point I was confused on and I am thinking you might be, as
well.

  1. The class name defined within the file will change. Instead of
    having a class named JLogEntry -- which translates to joomla\log\entry, the
    class will be named Entry. And that's good, because the file is named
    entry.php. (Although we might need to uppercase the first letter of those
    filenames.)

So, the top of the JLogEntry file will look like this:

namespace JPlatform\log;

class Log
{
etc.

If I wanted to use this class in another class, I would instantiate it as:

$class = 'JPlatform\log\Log';
$instance = new $class();

or I could define a use statement on the top of the page (don't recommend
doing it this way as overrides become difficult):

namespace in\my\class\folder;
use Jplatform\log\Log;

Then within a method:

$instance = new Log();

Is that your understanding, too? Or, are you thinking the old JLogEntry
class name will continue to be used for the class definition? (because that
is not the plan - and it was a point of confusion for me until Friday.)


Reply to this email directly or view it on GitHubhttps://github.com//issues/1787#issuecomment-11924638.

from joomla-platform.

dukeofgaming avatar dukeofgaming commented on August 16, 2024

Guys, I moved the existing Jira issue put by Don to an experimental workflow, I need feedback to "debug" the workflow. I'll put some details in the other mailing list post to not hijack this thread.

https://joomla.jira.com/browse/PLATFORM-4

from joomla-platform.

realityking avatar realityking commented on August 16, 2024

This all looks pretty good to me - except one thing: Performance of the compatibility layer is going to stink. It will load every single platform class. In an environment not using a bytecode cache (and probably even in those using one) that won't be nice - especially when you consider you'd still be doing that on cached pages when using the Joomla CMS cache.

I haven't thought much about it but I think we could use JLoader to out advantage here. If JLoader can't find a class it could look up the class in the array, define the alias and try loading again. I'm not a 100% sure that's a better or even a good idea, but I wanna put it out there so other can think about it too ;)

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

That's a good point Rouven. I had been trying to think of a way to
accomplish the compat layer without having to load every class first, and
you're suggestion gave me some ideas.

On Mon, Jan 7, 2013 at 5:31 PM, Rouven Weßling [email protected]:

This all looks pretty good to me - except one thing: Performance of the
compatibility layer is going to stink. It will load every single platform
class. In an environment not using a bytecode cache (and probably even in
those using one) that won't be nice - especially when you consider you'd
still be doing that on cached pages when using the Joomla CMS cache.

I haven't thought much about it but I think we could use JLoader to out
advantage here. If JLoader can't find a class it could look up the class in
the array, define the alias and try loading again. I'm not a 100% sure
that's a better or even a good idea, but I wanna put it out there so other
can think about it too ;)


Reply to this email directly or view it on GitHubhttps://github.com//issues/1787#issuecomment-11977533.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

While I'm in these files, I think it may be a good time to remove all the defined('JPATH_PLATFORM') checks in the class files. There really is no need for them. I mean, I understand their original purpose, but none of the platform files have any side effects other than declaring classes, constants, functions, and interfaces. Even IF they were accessed directly, there would be no way for them to execute anything.

What say you?

from joomla-platform.

realityking avatar realityking commented on August 16, 2024

It's not about executing some code but because the resulting error results in an information disclosure issue.

In a perfect world platform classes would never live in a web accessible space (there are framework with that assumption).

from joomla-platform.

elkuku avatar elkuku commented on August 16, 2024

E.e.
class A extends B
when class B isn't defined yet...
Using phars would make this obsolete ;)
On Jan 10, 2013 12:40 AM, "Rouven Weßling" [email protected] wrote:

It's not about executing some code but because the resulting error results
in an information disclosure issue.

In a perfect world platform classes would never live in a web accessible
space (there are framework with that assumption).


Reply to this email directly or view it on GitHubhttps://github.com//issues/1787#issuecomment-12081512.

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

I think we could think about changing the case of the folders and files in libraries/ to Camel Case.

If all files are affected by the namespacing, then it might be the right time.

  1. This is the general convention for most frameworks and packages that you can grab via composer.
    2 ) Faster autoloading because the namespace case matches the path case.
    3 ) Improve files and folders readability especially when it is composed of more than one part. For example instead of having controller/base.php we can have Controller/AbstractController.php

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

@florianv - see the compatibility layer gist for the proposed PSR-0 namespace structure. I'm currently working on this in my PlatformNamespace and have completed 12 of the 42 packages so far.

I'm leaving the current file / folder structure in tact to make merging interim PR's easier, until it's ready to merge. Once all the namespacing is done, I'm going to go back through and rename / move all the files to the new structure.

See the second bullet under Side Effects > Potentially Negative

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

Sorry, I didn't see this bullet.

It looks very good :) there is quite a bit of work. I will send you some pull requests.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Awesome. I'm sure you know this, but for others who'd like to contribute to
the effort, this is how you work on my PlatformNamespace branch.

As for the namespace process and how to do it, check out the work done in
the access through document packages so far to get an idea. I'll
document the ns process later.

On Thu, Jan 10, 2013 at 3:36 PM, Florian Voutzinos <[email protected]

wrote:

Sorry, I didn't see this bullet.

It looks very good :) there is quite a bit of work. I will send you some
pull requests.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1787#issuecomment-12120016.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

I've added a Contributing to the Effort section in the RFC. If you are interested in helping / getting things done, please check off what you'd like to be responsible and leave a comment that you are doing it, so we can track and not duplicate effort.

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

I can help you a little bit right now starting from the bottom packages 'view' etc

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

In the 2 hours since he posted that message, @florianv completely namespaced 8 packages AND gave me some pointers on an easier way to complete this task. Thanks for the help and awesome work!

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

We should add the task : 'creating a composer.json file in each package so they can be grabbed individually via composer'.

Once this is done we can accomplish this #1799 and add the dependency on the Psr log package in the Joomla log package.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

Slow down Florian :) Composer is another conversation we have not had yet. One step at a time please :)

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

ok :)

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

When namespacing some packages, we noticed that 2 class names are conflicting with php reserved keywords :

https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/joomla/log/logger/echo.php#L24

https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/joomla/html/list.php#L26

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

Given all the great work that has been started on this project, it became apparent there might be benefit to further defining the approach to namespacing so that the top of the page can be used at a glance to identify the requirements for the class (outside of the current namespace) and so that the code, itself, is not cluttered with namespace data. Don and I talked about this and agreed on some guidelines. Here's a link to a gist that helps clarify those guidelines.

https://gist.github.com/4520559

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

https://gist.github.com/452055 https://gist.github.com/45205599

I think that might be better presented as a new page in the Coding
Standards section of the manual. We should probably get into the habit of
using the MUST/SHOULD RFC as well.

Regards,
Andrew Eddie
http://learn.theartofjoomla.com - training videos for Joomla developers

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

I don't believe I can fork this RFC, can I? At least, I can't see how. Might have to have @dongilbert update the examples after downloading the gist.

As far as the Coding standards, I agree. @eddieajau Do you want that done now? Or, should we wait until we have the project somewhat complete, and have hopefully encountered all the various issues that might impact the standard? Either way, glad to help do that.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

I think it is better to draft the standards earlier than later. We can be
ratifying them while the coding effort works on in parallel.

Regards,
Andrew Eddie
http://learn.theartofjoomla.com - training videos for Joomla developers

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

Makes sense. @dongilbert if you want to include those gists as updates to the standard (I do believe they are likely more relevant than the early examples), you can download the gists and copy them into your RFC. If I can be of assistance with any of the writing, let me know. I don't believe I can fork it, or change it unless we use a different approach. I think Don can, tho.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

AAANNDDD all packages have now been namespaced. I'm going to work on the compatibility layer and testing.

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

Nice :) Which namespace are we using for the tests ? JoomlaTests ? With the current folder structure we cannot use Joomla\Tests.

We can if we have tests/Joomla/Tests/

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

@florianv When you get a chance, will you please review https://gist.github.com/4520559 and make modifications needed for the classes you had worked on? There is also a response I left on your PR that talked about issues that will need to be fixed. Ping me or Don if you have questions.

Thanks for your work guys!

from joomla-platform.

florianv avatar florianv commented on August 16, 2024

@dongilbert In https://github.com/dongilbert/joomla-platform/tree/PlatformNamespace/libraries/Joomla/Cache/Storage

Memcached and Memcache have an extra e : Memecached

@AmyStephen The examples look good :)

To solve the name issues we can suffix/prefix the classes in the packages :

Cipher3DES, EchoLogger

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Thanks Florian. You can send a PR or I'll get to it later.

For the tests, I'm not wanting to rewrite the tests to use the new class names yet. We need to test the compatibility layer, so I'm currently trying to get all the tests to work using that instead. If all the tests can pass, then I can be confident was have a solid foundation to start with there.

Sent from my iPhone

On Jan 14, 2013, at 2:49 AM, Florian Voutzinos [email protected] wrote:

@dongilbert In https://github.com/dongilbert/joomla-platform/tree/PlatformNamespace/libraries/Joomla/Cache/Storage

Memcached and Memcache have an extra e : Memecached

@AmyStephen The examples look good :)


Reply to this email directly or view it on GitHub.

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

What a huge job you took on @dongilbert Much appreciated.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

@florianv, I updated the cache file names. Thanks for pointing that out.

The current state of testing is that it's actually completing ~ 65% of the
tests. There's a lot of issues with test classes including their own copies
of the files via "include_once" which doesn't work, since the file
structure changed. Some of the tests that are completing are still failing,
but I don't see it as being a huge problem to figure out once I sort out
the fatal errors due to bad testing practices. :)

Thanks @AmyStephen. 👍

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Update: Now able to run all the tests, 100%. There are 51 failures and 140 errors. Shouldn't take long to fix. :)

http://d.pr/i/MjoJ

Edit: I messed up a commit, used git reflog to rewind to a commit, and then deleted the branch, losing all the changes to test. :( NBD, just gotta go through them all again and remove the include_once and require_once calls. Again. arggghh

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

:(

Well, at least the pay is good!

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

And we're back. :)

Most failures now are because of things like this: http://d.pr/i/og5W

44) JHtmlBehaviorTest::testCalendar
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    'JHtmlBehavior::calendar' => true
+    'Joomla\Html\Behavior::calendar' => true
 )

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

Well, I guess those aren't a big surprise, huh? ;-) If you can hold off until this evening, and if Florian hasn't looked at the examples and my earlier feedback so that he can redo his classes, I'll help get those fixed.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

I've worked out all the 140 errors. It pointed out some places where I messed up the namespacing or imported the wrong class, so that's good.

We're currently at 13 failures and 0 errors. Those are all just referencing old class names, so - no big deal!

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

You are single-handedly taking care of things these days.

Thanks very much @dongilbert

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Thank you Amy. Another update: 0 failures or errors! YA!

I wouldn't say single-handedly. @florianv played a big part in it. 👍

There are a few hacks where classes are being declared twice that I have to take care of, but other than that, we are golden. The test suites are mostly running entirely on the old class naming structure just fine via class_alias. The only areas that were failing and I had to replace with the new class names were where it was directly comparing names via get_class and when the test was asserting that a class was an instanceof an old class name. It would be working, except I don't load in and alias all the old class names any more, I only alias as needed. Thanks to @realityking for that idea.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

All testing problems resolved. There was an issue with my PSR-0 classloader that wasn't exactly playing nice. We're all good now.

The reason I built a PSR-0 classloader instead of using a default one is that we have some special cases what with the need to class_alias all the old style class names. I'm going to commit all my changes and push for testing by others.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

I've added a loadByPsr0 method to JLoader that handles loading all the platform classes as well as any 3rd party vendor code that you may include in your application. You register a namespace the same way you would with the previous loadByNamespace* methods, with one significant difference. The loadByNamespace* methods always removed the root namespace when searching from the filesystem. They function exactly like the prefix register and autoload method. PSR-0 does NOT remove any part of the namespace when loading from the filesystem. So, the difference is as follows:

/**
 * Registering a namespace for old namespace loaders.
 *
 * When loadByNamespaceMixedCase is called, it strips `Joomla`
 * from the namespace, and then builds the file path. So when registering
 * the namespace, you need to include that path in your register call.
 */
JLoader::registerNamespace('Joomla', JPATH_PLATFORM . '/joomla');

/**
 * Registering a namespace for PSR-0 loader.
 *
 * The main difference is you don't include the folder that matches
 * the namespace you are registering, because when PSR-0 transforms
 * the class name into a file lookup path, it doesn't remove any part.
 */
JLoader::registerNamespace('Joomla', JPATH_PLATFORM);

The downside of the first approach is the advantage of the second. With the first approach, you cannot register sub namespaces, only root ones. registerNamespace('Joomla\Platform', JPATH_PLATFORM); would register, but it would fail to load the file from the file structure, because it would loop through JLoader::$namespaces looking for the Joomla root namespace, and never find it.

You can find the proposed PSR-0 autoloader here - https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/loader.php#L592

@realityking You can see how I used your approach to aliasing class names via JLoader, instead of an all in one shot right here - https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/loader.php#L571

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Another update: I've been testing the namespaced platform more by replacing the libraries/joomla folder and the libraries/loader.php file of the JTracker app with their namespaced counterparts. There were a few hiccups that showed me some slight modifications to the loader that needed to be made, but overall, the system is stable.

The part that was broken was overriding the new namespaced classes with an old style override. It's a bit of a hack with the autoloader, but it makes sense, and it works beautifully. I'll count that as a win. See the updated compatLayer method here: https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/loader.php#L571

from joomla-platform.

AmyStephen avatar AmyStephen commented on August 16, 2024

Don - do you see any value in creating a collection of "reserved" aliases? laravel/framework#95

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

There may be, I know FuelPHP took this approach with some things. You could override a core class by naming your own and then registering it in the bootstrap. You can do that now in J! if you copy the class to the new destination and then register it to the autoloader. However, this circumvents doing cool things with namespaces and such, so that, instead of copying the core class, you can have your class extend and then override, but still call the same class in code.

App::alias('Auth', 'my/custom/auth.php');

// auth.php

namsespace My\Custom;

class Auth extends \Joomla\Authentication\Auth {}

Then, the rest of the app uses your custom auth as the auth class instead. It is useful, and it doesn't require copying entire classes to a new file, you can simple extend and override.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

2 months until the scheduled release of 13.1. This completed code is looking really solid; I've replaced the libraries/joomla folder of JTracker and the app remained solid. I'm going to start testing on a CMS level and do the same thing. I don't expect any issues, but if there are, they should be relatively easy to fix.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

Don, you probably need to add another check box to the task list and that
to polish and expand "the pitch" (the RFC text). I think we need quite a
few examples of how this change will impact the code (that should be
relatively easy to pull out snippets from the actual changes, but we just
need to home in on typical "simple" and atypical "complex" cases); how
someone in the CMS (if the CMS choses to embrace the NS'd platform) might
leverage it alongside non-NS'd code; how overrides will work in practice;
etc.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Ok - That's next on my todo list for moving this forward.

from joomla-platform.

eddieajau avatar eddieajau commented on August 16, 2024

I think we can safely say we have this in hand now.

from joomla-platform.

dongilbert avatar dongilbert commented on August 16, 2024

Certainly.

On Mon, Mar 4, 2013 at 3:01 PM, Andrew Eddie [email protected]:

I think we can safely say we have this in hand now.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1787#issuecomment-14405212
.

from joomla-platform.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.