Issues

ZF-10795: Zend_Http_UserAgent_Features_Adapter_WurflApi does not properly pass in configuration paramters to Wurfl library

Description

In {{Zend_Http_UserAgent_Features_Adapter_WurflApi}}, the ZF documentation states that a parameter may be used to configure the wurfl adapter with an array using the {{wurfl_config_array}} key as part of the UserAgent configuration. The array is passed to the wurfl configuration in the code block below:

$wurflConfig->wurflFile($c['wurfl']['main-file'])
    ->wurflPatch($c['wurfl']['patches'])
    ->persistence($c['persistence']['provider'], $c['persistence']['dir']);

There are a couple issues with this. 1. It does not allow configuration of the "cache provider" for the wurfl library, something that would be nice to have and differs from the "persistence" functionality that is configured here. 2. It does not pass in the {{dir}} parameter in correctly for the "persistence" provider. It should be passed as an array with the {{dir}} key specified instead of passing the {{dir}} parameter in directly. 3. It assumes that we are using a persistence provider that requires a {{dir}} parameter, which could very well not be the case, like with Memcache or some other adapter.

For example, to get the wurfl persistence provider to use the {{dir}} parameter from a Zend_Config_Xml, one would have to structure it like this:


file
/home/jray/htdocs/newui/application/cache/wurfl/persistence

A better implementation that would also allow for arbitrary parameters to be passed would be something like this:


file
/home/jray/htdocs/newui/application/cache/wurfl/persistence

If the code in {{Zend_Http_UserAgent_Features_Adapter_WurflApi}} was changed to something like this it would resolve all of the issues above.

$wurflConfig->wurflFile($c['wurfl']['main-file'])
    ->wurflPatch($c['wurfl']['patches'])
    ->persistence($c['persistence']['provider'], $c['persistence']['params'])
    ->cache($c['cache']['provider'], $c['cache']['params']);

Comments

This patch includes the change mentioned in the description.

@Jared: Patch needs unit tests to confirm behavior (i.e. 'dir' is currently broken as described in the docs) and then apply your fix and the test should pass. I'm not familiar with this component, but what if the 'cache' parameter is not passed? Also need tests to ensure failing to provide these optional params doesn't break something.

Also is a documentation patched needed to explain the new ability to pass cache information this way?

The cache section should be checked to see if it exists, but the ->persistence() portion probably needs to at least be pulled off into a separate ticket and fixed immediately given that it a bug (and not a missing feature like cache).

Actually there's a secondary bug within WURFL itself, while clearly the config should act as described by Jared, WURFL itself has no idea what it wants (WURFL/Xml/PersistenceProvider/PersistenceProviderManager.php line 54).

Zend\Http\UserAgent\Features\Adapter\WurflApi is in my opinion unfixable for a WURFL 1.1 implementation due to inconsistencies in the WURFL API Config classes.

If we pass the config_array in with the 'dir' param instead of 'params' the API will throw a WURFL_WURFLException with the message: Specify a valid Persistence dir in the configuration file

This is due to the WURFL_Configuration_InMemoryConfig class setting the persistence array with two keys, 'provider' and 'params' (which is the expected use case), however the WURFL_Xml_PersistenceProvider_FilePersistenceProvider initialize function expects the persistence array to have a key of 'dir' which it will never have due to the keys being set forcefully in InMemoryConfig->persistence().

My suggestion would be to remove the ability to configure Wurfl using a wurfl_config_array in Zend\Http\UserAgent\Features\Adapter\WurflApi using WURFL 1.1, and add a switch case for WURFL 1.2 allowing it as the bug has been fixed in this WURFL release.

I have attached a patch to this affect.

Also included in this patch is changes to the UserAgentTest file to remove deprecated assertion assertType and replace it with assertInstanceOf

Daniel and Rob are correct about the bug in WURFL 1.1. I was using the WURFL 1.2 library and the bug is fixed in version 1.2. I agree with Daniel that the cache section should be checked to see if it exists in the configuration.

For the moment it is possible to use WURLF 1.2.1 with following configuration without altering the code: 'wurflapi' => array ( 'wurfl_api_version' => '1.1', 'wurfl_lib_dir' => BASE_PATH .'/library/WURFL/', 'wurfl_config_array' => array ( 'wurfl' => array ( 'main-file' => $resourcesDir .'wurfl-2.0.27.zip', 'patches' => $resourcesDir .'web_browsers_patch.xml', ), 'persistence' => array ( 'provider' => 'file', 'dir' => array('dir' => $cacheDir), ), ) ),

I didn't know about this bug and a bug I created today appears to be a duplicate. However after reading through this bug I believe that the fix in this bug report would break anyone's code who has used the workaround mentioned in my bug report and that mentioned on the stackoverflow.com article.

The fix I have suggested is backward compatible:

http://framework.zend.com/issues/browse/ZF-11743

Note: My patch also takes into account any other provider/adapter that does not have a "dir" parameter but may have other parameters.

Wrong issue linked above, corrected link to "Zend_Http_UserAgent_Features_Adapter_WurflApi Sends Persistence Information using WURFL_Configuration_InMemoryConfig to WURFL_WURFLManagerFactory incorrectly"

[http://framework.zend.com/issues/browse/ZF-12284]