CMS Made Simple Forums • Don't use the internals CMSMS in your code.

Page 1 of 2

Don't use the internals CMSMS in your code.

Posted: Thu Sep 16, 2010 3:30 pm

by calguy1000

Due to the humble beginnings of CMSMS in the php4 world, all of the member variables for the various objects were declared public (php4 didn't have the concept of private and protected member variables).

As a result, there are alot of objects in CMSMS that still have public member variables, and as design goes this is a bad thing. A good developer knows that access to the data should be controlled so that you can manage the integrity of the data, and make sure it's valid for all of the other objects that may use that data.

However we can't go through and 'change' all of this stuff in CMSMS en-mass (as much as we'd like to) for a number of reasons:
a) It'd break alot of stuff in CMSMS, and cost a lot of time to test and fix everything
b) It'd break alot of external modules, plugins and UDT's.
c) We don't have the API's for everything yet, to replace the stuff that would be made inaccessible.

So here are the guidelines:
a) Use the accessors and API's wherever possible
i.e: don't include config.php yourself.

b) Any member variables in any of the CMSMS supplied objects should be considered PRIVATE.
For example, there are a few member variables in $gCms that people are accessing all the time.
i.e: $gCms->variables and $gCms->modules, and a few others
These will be made private, or may be re-arranged over time to something different. They
should be considered CMSMS internal variables, and you should not use them.

We may need to write some further API functions to make some things easier. But for now
consider this your warning.

c) Objects or arrays returned from CMSMS methods, unless stated otherwise should be considered
READ ONLY
i.e this is okay:

Code: Select all

        
        global $gCms;
        $config = $gCms->GetConfig();
        $foo = $config['root_path'].'/foo';

this is not:

Code: Select all

        
        global $gCms;
        $config = $gCms->GetConfig();
        $config['root_path'] .= '/foo';

d) If there is something you need to pull from CMSMS, and can't see an API to do it, ask a clearly explained, well researched question, and we will help you. We won't be responsible for, or support your plugin/module/UDT not working in future versions of CMSMS because you were using internal private data for CMSMS after being warned.

Now that does not mean that I'm gonna go whole hog and mark a bunch of stuff private in 1.9.. but it does mean that you now have the opportunity to
a) submit suggestions as to what is missing in the API
b) change your code to work properly so that hopefully it will have longevity.

Re: Don't use the internals CMSMS in your code.

Posted: Thu Sep 16, 2010 5:03 pm

by Nullig

So, are you saying I shouldn't use:

$modules = array_keys($gCms->modules);

Do I need to do a db lookup?

Nullig

Re: Don't use the internals CMSMS in your code.

Posted: Thu Sep 16, 2010 5:10 pm

by calguy1000

I'm saying that we'll have to write some API functions for stuff like this.

Modules in particular.

I think I'll add something like CmsModule::get_module($name,$version);

Re: Don't use the internals CMSMS in your code.

Posted: Mon Sep 20, 2010 5:32 pm

by NaN

Will there be a way to access properties of the content object?
Here a user asked how to access the secure property of pages in templates.
Since the content object is a template var this might be no problem.
But what about udts or plugins?

Anyway in some templates i don't use the content tag to display the page content.
It is just in the template because it needs to be there to work properly but it is assigned to a var and not used.
Instead of the var is use the content properties of the $content_obj to loop through them and create a pretty dynamic template.
Example:

Code: Select all


{content assign="content"}

{* This is commented out since i only need this in backend to show the content blocks

    {content block="block1"}
    {content block="block2"}
    {content block="block3"}
     ...

*}

{foreach from=$content_obj->mProperties->mPropertyValues item=prop_value key=prop_name}
    {if ... some smarty stuff to check the entered values}

        {$prop_value}

    {else}

        ...
        or even do this:
        {assign var="prop_title" value=$prop_name|cat:'_title'}
        {$content_obj->mProperties->mPropertyValues.$prop_title}

    {/if}
{/foreach}

Is there a way to do this without accessing the private vars?
(even with a UDT or a plugin - doesn't need to be done in the template itself)

Re: Don't use the internals CMSMS in your code.

Posted: Mon Sep 20, 2010 5:38 pm

by calguy1000

The content object is an object with properties and accessors.
The properties should be considered private.

This stuff already exists.

Code: Select all

{if $content_obj->secure()}this page is secure{/if}

should work perfectly.

In a UDT there's the ContentOperations stuff that can be used to grab a page.
The current content object is stored in $gCms->variables
and we can add a convenience method somewhere to grab it.

Re: Don't use the internals CMSMS in your code.

Posted: Mon Sep 20, 2010 5:48 pm

by NaN

calguy1000 wrote:
In a UDT there's the ContentOperations stuff that can be used to grab a page.
The current content object is stored in $gCms->variables and we can add a convenience method somewhere to grab it.

Good news

Re: Don't use the internals CMSMS in your code.

Posted: Sat Sep 25, 2010 3:18 pm

by calguy1000

as another note.

$gCms should be considered deprecated. for the best chances of forward compatibility please use the cmsms() function.

This one is gonna kill me. I use this everywhere.

Re: Don't use the internals CMSMS in your code.

Posted: Sat Sep 25, 2010 6:37 pm

by NaN

So all plugins, modules, tags, docs, wikis ... needs to be updated and change

Code: Select all

global $gCms;

in

Code: Select all

$gCms = cmsms();

?

Sounds like much work... $:-\$

I wonder if there is a convenient method to get the username of the currently logged in backend user. Currently i'm using

Code: Select all

$gCms->variables['username']

what seems to be no good idea anymore.

In the file lib/page.funtions.php there is a function get_userid() that just returns the userid of the logged in user by returning a session var. But i found nothing similar for the username. Do i really need to query the database again using this one:

Code: Select all


$gCms = cmsms();
$userops =& $gCms->GetUserOperations();
$user = $userops->LoadUserByID(get_userid());
$username = $user->username;

?

Re: Don't use the internals CMSMS in your code.

Posted: Sat Sep 25, 2010 7:35 pm

by calguy1000

Well you're really talking about two different things:

First, You're saying you 'found' some code within CMSMS that caches the username. and you are taking advantage of that cached username.

We both know that that isn't a great 'going forward' solution, because that isn't a public API. Infact it isn't documented as something that is allowed anywhere... and therefore the developer of that code is free to change it, to do something different and if that change breaks your code it couldn't be called a bug.

The next thing you're talking about is the efficiency of a particular method or methods that you want to use.

Every object and method in CMSMS has a history, it was designed for a certain purpose... and yes, we could change the method to cache the results. But then we have to analyze why... because we are then trading memory requirements for SQL queries. And in this particular method... would one extra query really hurt? How many users are you going to be requesting? can't you cache the username yourself? is it a bug? I don't think so. Something that needs improvement? that depends on your point of view.

Or are you just after a 1 line solution?

Re: Don't use the internals CMSMS in your code.

Posted: Sat Sep 25, 2010 7:59 pm

by NaN

No i'm not just after a 1 line solution.
Just wanted to know if the code i posted is a valid (and the only one) solution to get the username.

Re: Don't use the internals CMSMS in your code.

Posted: Sat Sep 25, 2010 8:03 pm

by calguy1000

yeah, currently:

Code: Select all

cmsms()->GetUserOperations()->LoadUserById(get_userid())->username

is about the only solution. I could easily write something for that... but then, if it's important to you, just write a convenience function for it.

As far as the $gCms vs cmsms() stuff goes... yeah it is a pain in the ass... but it's only 20 minutes of work per module, and we're not removing it from 1.9 ... just deprecating it... so you have some notice.

Re: Don't use the internals CMSMS in your code.

Posted: Sat Sep 25, 2010 8:17 pm

by calguy1000

Okay.

As of today, we have a 'kitchen sink' class.. of sorts. a place where convenience methods are stored, stuff that is used often within the core and within a large chunk of modules. This new static class is called cms_utils

Code: Select all

$content_obj = cms_utils::get_current_content();
if( is_object($content_obj) )
  {
      echo "Alias is: ".$content_obj->Alias();
  }

or

Code: Select all

$feu = cms_utils::get_module('FrontEndUsers','1.10');
if( is_object($feu) )
  {
      $user_id = $feu->LoggedInId();
  }

---
ALSO:

There are methods in the cms_utils class called set_app_data() and get_app_data() designed to allow you to store data globally, for use in multiple module calls... without interfering with CMSMS internals.

---
ALSO

The config array (i.e: $gCms->GetConfig()) has been changed to an object. And it WILL generate an ERROR if you try to set or UNSET a config variable. It's usage is exactly the same however (I love ArrayAccess)I don't expect that this will cause many problems, because if it is causing a problem with your module, then well.. your module is poorly behaved, and is trying to effect the behavior of other modules.

---
ALSO:

We are experimenting with changing $gCms->variables to an object similar to the config object, instead of a flat array. If we commit this, it will NOT generate errors for bad modules, but will generate NOTICES, indicating which modules are behaving badly, and should be changed.

Re: Don't use the internals CMSMS in your code.

Posted: Sun Sep 26, 2010 4:59 pm

by calguy1000

Okay, I've now replaced the $gCms->variables array with an object.
Only certain properties can be set. Attempting to set/unset other variables will generate a notice so that module developers can fix thier code.

cms_utils::set_app_data('key',$value); is now the best way to store data for your module that may be needed across module requests.

Though, you could also store it in your module class.

Re: Don't use the internals CMSMS in your code.

Posted: Mon Sep 27, 2010 9:59 am

by Peciura

"cms_utils" - lovely.

Re: Don't use the internals CMSMS in your code.

Posted: Mon Oct 11, 2010 1:28 pm

by bess

my English is not well enough to understand all the subtlety of this discussion, so I ask you a bit of patience with my questions.

I understood that V1.9 will bring a lot of changes in the methods declared deprecated (and rightly so)

I have in the elbow a few examples of uses of the current API and I do not know after reading all these messages if I start modifying my code or not

supplementary question: Now modify the code, this is not likely to prevent any compatibility with versions V1.8 and earlier?

in class Foo

Code: Select all

class Foo extends CMSModule

Which is better?

Code: Select all

$this->GetPreference("fooPref");
$this->SetPreference("fooPref", 'new value');

or

Code: Select all

cms_utils::get_app_data('fooPref');
cms_utils::set_app_data('fooPref','new value');

Which is better?

Code: Select all

$feu = &$gCms->modules["FrontEndUsers"]['object'];

or (i suppose :p)

Code: Select all

$feu = cms_utils::get_module('FrontEndUsers','1.10');

and for

Code: Select all

$db =& $gCms->GetDb();

do we must done something ?

thank you in advance for helping me

mon anglais n'est pas assez bien pour comprendre toutes la subtilité de cette discussion, aussi je vous demanderais un peu de patience avec mes questions.

J'ai compris que la V1.9 va apporter son lot de modifications au niveau des méthodes déclarées deprecated (et c'est très bien ainsi)

J'ai sous le coude quelques exemples d'utilisations de l'API actuelle et je ne sais pas après lecture de tous ces messages si je dois commencer à modifier mon code ou pas

question complémentaire : modifier le code MAINTENANT, n'est ce pas le risque d'empêcher toute compatibilité avec les versions V1.8 et précédente ?