View Issue Details

IDProjectCategoryView StatusLast Update
0026740mantisbtplug-inspublic2020-03-03 03:13
Reporterval-kulkov Assigned To 
PrioritynormalSeverityfeatureReproducibilityalways
Status newResolutionopen 
Product Version2.23.0 
Summary0026740: plugin_require_api() does not seem to serve any practical purpose
Description

The official PHP documentation for function include, which also applies to the function require, states the following (emphasis added):

If the include occurs inside a function within the calling file, then all of the code contained in the called file will behave as though it had been defined inside that function. So, it will follow the variable scope of that function. An exception to this rule are magic constants which are evaluated by the parser before the include occurs.

Therefore, require_once() on line 826 in plugin_api serves no practical purpose. Remote PHP debugging shows that this require_once() properly imports variables and functions from $p_file, but then all imported variables and functions are lost as soon as the scope of function plugin_require_api() is lost.

I am not suggesting a fix just yet, because the purpose of plugin_require_api() is not clear to me. I would appreciate a clarification. If this function is supposed to import variables and functions from $p_file into the plugin's namespace, then this function should either return the imported variables and functions or provide a constructed filename so that the caller can execute require_once() directly. If, on the other hand, plugin_require_api() is supposed to use some magic constants, then it is entirely unclear how this process should work.

I am writing a Mantis plugin which would use lang_api to import plugin error messages in a language chosen by the user. At present, plugin_api.php provides a stub function errors() to import error messages, allowing the plugin implementation to override it. Unfortunately, there is no easy mechanism at present to import error messages in a language chosen by the user: see lines 1002-1007. Importing error messages via plugin_require_api() does not work because, as described above, the variables are lost as soon as the scope of plugin_require_api() is lost.

For now, I am using the following code in the plugin's register() function:

# Plugin error messages
$t_current = plugin_get_current();
$t_path = config_get_global( 'plugin_path' ) . $t_current . '/';
$t_plugin_errors_file = $t_path . 'lang/errors_' . lang_get_current() . '.php';
require_once( $t_plugin_errors_file );
$this->error_text = $plugin_errors;

and then I define errors() like this:

function errors() {
    return $this->error_text;
}

but then again, I wonder what the purpose of plugin_require_api() is.

TagsNo tags attached.

Activities

cproensa

cproensa

2020-02-25 15:21

developer   ~0063692

Here you can see an example of using the plugin errors() function:
https://github.com/mantisbt-plugins/timetracking/blob/6535f2d8ee70ea80990d532d679b8b45fd96a9d8/TimeTracking/TimeTracking.php#L82-L87
where error codes are defined as constants in the plugin namespace, and error strings are retrieved from the plugin translation files.

the purpose of plugin_require_api() is not clear to me

plugin_require_api()has the benefit of inferring the plugin path, so you can specify a file relative to the plugin root. For example:
plugin_require_api( 'core/constants.php' ); for a file within a directory "core" under the plugin directory structure.

One thing that plugin_require_api does different from require_api is that it does not register the global variables, but that can be replicated with the similar code to:
https://github.com/mantisbt/mantisbt/blob/a1f45450accd9ccd7d93dffbed085d388fe37d75/core.php#L120-L123

I'm not sure if that should be changed, or adding globals to all the scope is a risk of overwritting other globals defined in the core includes.
So far, it hasn't been a problem for me, as i don't use globals across includes when writting plugincode.

I am using the following code in the plugin's register() function:

I suggest not to include other files like that in register(). Because register is always called as part of plugin discovery, even if the plugin is not enabled.
A better place is init() that is only called for a plugin that is enabled.
An exception would be the linked code above, where a constants file is needed to be used also within register().

For some more involved plugins, my current approach is to use namespaced classes, and an autoloader, so the whole plugin_require_api is not needed.

dregad

dregad

2020-02-26 05:00

developer   ~0063697

To my knowledge, plugin_require_api() is used to include a plugin-specific API (i.e. a series of functions) without having to worry about the actual plugin path. I have only seen it used in MantisGraph core plugin, and I don't think it was intended to register variables in the global namespace and like @cproensa I'm not sure it should.

Note that the scoping issue only affects variables declaration - PHP functions are always global [1]

All functions and classes in PHP have the global scope - they can be called outside a function even if they were defined inside and vice versa.

As for registering error messages, another example can be found in Source Integration plugin (https://github.com/mantisbt-plugins/source-integration/blob/v2.3.1/Source/Source.php#L125)

val-kulkov

val-kulkov

2020-03-02 14:48

reporter   ~0063720

Thank you @cproensa and @dregad for your valuable comments. I changed my approach with plugin_require_api() and I am now trying to use namespaces as much as I can to keep plugin names away from the Mantis namespace. It is indeed a safer approach.

Since I started using namespaces, I found that apparently I cannot use plugin_event_hook to register namespaced functions. For example:

plugin_event_hook( 'EVENT_LAYOUT_RESOURCES', 'add_resources_v_list' );

works, but

plugin_event_hook( 'EVENT_LAYOUT_RESOURCES', 'MyPlugin\add_resources_v_list' );

does not.

Interestingly, if I define a function and then call plugin_event_hook() with this function in a plugin's page, the function never gets called. The result is the same regardless of whether I use a separate namespace or not: the function never gets called. It appears that in order for plugin_event_hook() to work with a function, the function must be defined inside the plugin's class definition.

Since the second argument of plugin_event_hook() is a string, clearly the function with the name matching this string must be accessed dynamically in the PHP code. Why such dynamic access fails to work with namespaced functions or functions defined within a plugin's page code is somewhat a mystery to me. At this point, I am not ready to dig deeply into the gory details of the Mantis code to understand why this happens, so my comments are simply an observation of this behaviour which seems odd and potentially misleading to me.

If this behaviour is the intended behaviour, should there be some comments in the plugin_event_hook() code to guide developers to use only functions defined in the plugin's class definition to avoid problems similar to mine?

dregad

dregad

2020-03-03 03:13

developer   ~0063721

It appears that in order for plugin_event_hook() to work with a function, the function must be defined inside the plugin's class definition.

@val-kulkov it may be poorly documented, but plugin hooks are indeed expected to be methods of the plugin's base class.

Technically, event_hook() API function allows you to hook a regular function to an event (with $p_plugin paramer to 0) but I don't think this is actually used anywhere.

I am not ready to dig deeply into the gory details of the Mantis code to understand why this happens

Look at event_callback() function.