Some more test work

- Refactoring to reduce duplication
- Add tests for filesystem StreamWrapper
- Clear infolog's access cache

api/src/Vfs/Base.php

@@ -12,6 +12,7 @@
 
 namespace EGroupware\Api\Vfs;
 
+use EGroupware\Api\Config;
 use EGroupware\Api\Vfs;
 
 /**
@@ -133,7 +134,7 @@ class Base
 
 		if ($persitent_mount)
 		{
-			Api\Config::save_value('vfs_fstab',self::$fstab,'phpgwapi');
+			Config::save_value('vfs_fstab',self::$fstab,'phpgwapi');
 			$GLOBALS['egw_info']['server']['vfs_fstab'] = self::$fstab;
 			// invalidate session cache
 			if (method_exists($GLOBALS['egw'],'invalidate_session_cache'))	// egw object in setup is limited
@@ -164,7 +165,7 @@ class Base
 		}
 		unset(self::$fstab[$path]);
 
-		\EGroupware\Api\Config::save_value('vfs_fstab',self::$fstab,'phpgwapi');
+		Config::save_value('vfs_fstab',self::$fstab,'phpgwapi');
 		$GLOBALS['egw_info']['server']['vfs_fstab'] = self::$fstab;
 		// invalidate session cache
 		if (method_exists($GLOBALS['egw'],'invalidate_session_cache'))	// egw object in setup is limited

api/tests/LoggedInTest.php

@@ -79,6 +79,9 @@ abstract class LoggedInTest extends TestCase
 		// Reset stream context, or current user will always be there
 		stream_context_set_option(stream_context_get_default(),['vfs'=>['user' => null]]);
 
+		// Clear some link caching
+		Link::init_static(true);
+
 		if($GLOBALS['egw'])
 		{
 			if($GLOBALS['egw']->session)

api/tests/Vfs/Filesystem/StreamWrapperTest.php

@@ -0,0 +1,58 @@
+<?php
+
+/**
+ * Test the basic Vfs::StreamWrapper
+ *
+ * @link http://www.egroupware.org
+ * @author Nathan Gray
+ * @copyright (c) 2020  Nathan Gray
+ * @license http://opensource.org/licenses/gpl-license.php GPL - GNU General Public License
+ */
+
+namespace EGroupware\Api\Vfs\Filesystem;
+
+require_once __DIR__ . '/../StreamWrapperBase.php';
+
+use EGroupware\Api;
+use EGroupware\Api\Vfs;
+
+
+class StreamWrapperTest extends Vfs\StreamWrapperBase
+{
+	public static $mountpoint = '/test/filesystem';
+
+	protected function setUp() : void
+	{
+		parent::setUp();
+
+		$this->files[] = $this->test_file = $this->getFilename();
+	}
+
+	protected function tearDown() : void
+	{
+		parent::tearDown();
+	}
+
+	protected function  mount(): void
+	{
+		$this->mountFilesystem(static::$mountpoint);
+	}
+
+	protected function allowAccess(string $test_name, string $test_file, int $test_user, string $needed) : void
+	{
+		// We'll allow access by putting test user in Default group
+		$command = new \admin_cmd_edit_user($test_user, ['account_groups' => array_merge($this->account['account_groups'],['Default'])]);
+		$command->run();
+
+		// Add explicit permission on group
+		Vfs::chmod($test_file, Vfs::mode2int('g+'.$needed));
+	}
+
+	/**
+	 * Make a filename that reflects the current test
+	 */
+	protected function getFilename($path = null)
+	{
+		return parent::getFilename(static::$mountpoint);
+	}
+}

api/tests/Vfs/Links/StreamWrapperTest.php

@@ -25,7 +25,6 @@ class StreamWrapperTest extends Vfs\StreamWrapperBase
 	{
 		parent::setUp();
 
-		$this->mountLinks('/apps');
 	}
 
 	protected function tearDown() : void
@@ -65,13 +64,18 @@ class StreamWrapperTest extends Vfs\StreamWrapperBase
 		parent::testWithAccess();
 	}
 
-	protected function allowAccess(string $test_name, string $test_file, int $test_user, string $needed)
+	protected function allowAccess(string $test_name, string $test_file, int $test_user, string $needed) : void
 	{
 		// We'll allow access by putting test user in responsible
 		$so = new \infolog_so();
-		$element = $so->read($this->entries[0]);
+		$element = $so->read(Array('info_id' => $this->entries[0]));
 		$element['info_responsible'] = [$test_user];
-		$so->write($so);
+		$so->write($element);
+	}
+
+	protected function mount() : void
+	{
+		$this->mountLinks('/apps');
 	}
 
 	/**

api/tests/Vfs/StreamWrapperBase.php

@@ -21,7 +21,7 @@ use EGroupware\Api\Vfs;
 use EGroupware\Stylite\Vfs\Versioning;
 
 
-class StreamWrapperBase extends LoggedInTest
+abstract class StreamWrapperBase extends LoggedInTest
 {
 	/**
 	 * How much should be logged to the console (stdout)
@@ -84,6 +84,7 @@ class StreamWrapperBase extends LoggedInTest
 		{
 			$this->markTestSkipped('No write access to files dir "' .$GLOBALS['egw_info']['server']['files_dir'].'"' );
 		}
+		$this->mount();
 	}
 
 	protected function tearDown() : void
@@ -249,9 +250,11 @@ class StreamWrapperBase extends LoggedInTest
 		// Log in as them
 		$this->switchUser($this->account['account_lid'], $this->account['account_passwd']);
 
+		$this->mount();
+
 		// Check the file
-		$post = Vfs::stat($file);
+		$this->assertFalse(
-		$this->assertEquals(null,$post,
+				Vfs::is_readable($file),
 				"File '$file' was accessible by another user who had no permission"
 		);
 		$this->assertFalse(
@@ -305,6 +308,8 @@ class StreamWrapperBase extends LoggedInTest
 		// Log in as them
 		$this->switchUser($this->account['account_lid'], $this->account['account_passwd']);
 
+		$this->mount();
+
 		// Check the file
 		$post = Vfs::stat($file);
 		$this->assertNotNull($post,
@@ -386,6 +391,26 @@ class StreamWrapperBase extends LoggedInTest
 		return $path . $reflect->getShortName() . '_' . $this->getName() . '.txt';
 	}
 
+	/**
+	 * Mount the needed filesystem
+	 *
+	 * This may be called multiple times for each test as we change users, logout, etc.
+	 */
+	abstract protected function mount() : void;
+
+	/**
+	 * Allow access to the given file for the given user ID
+	 *
+	 * Using whatever way works best for the mount/streamwrapper being tested, allow the user access
+	 *
+	 * @param string $test_name
+	 * @param string $test_file
+	 * @param int $test_user
+	 * @param string $needed r, w, rw
+	 * @return mixed
+	 */
+	abstract protected function allowAccess(string $test_name, string $test_file, int $test_user, string $needed) : void;
+
 	/**
 	 * Mount the app entries into the filesystem
 	 *

api/tests/Vfs/StreamWrapperTest.php

@@ -24,6 +24,7 @@ class StreamWrapperTest extends StreamWrapperBase
 	protected function setUp() : void
 	{
 		parent::setUp();
+		$this->files[] = $this->test_file = $this->getFilename();
 	}
 
 	protected function tearDown() : void
@@ -33,21 +34,6 @@ class StreamWrapperTest extends StreamWrapperBase
 		parent::tearDown();
 	}
 
-	public function testSimpleReadWrite(): string
-	{
-		$this->files[] = $this->test_file = $this->getFilename();
-
-		return parent::testSimpleReadWrite();
-	}
-
-
-	public function testNoReadAccess(): void
-	{
-		$this->files[] = $this->test_file = $this->getFilename();
-
-		parent::testNoReadAccess();
-	}
-
 	public function testWithAccess() : void
 	{
 		// Put it in the group directory this time so we can give access
@@ -56,7 +42,12 @@ class StreamWrapperTest extends StreamWrapperBase
 		parent::testWithAccess();
 	}
 
-	protected function allowAccess(string $test_name, string $test_file, int $test_user, string $needed)
+	protected function mount(): void
+	{
+		// Nothing here
+	}
+
+	protected function allowAccess(string $test_name, string $test_file, int $test_user, string $needed) : void
 	{
 		// We'll allow access by putting test user in Default group
 		$command = new \admin_cmd_edit_user($test_user, ['account_groups' => array_merge($this->account['account_groups'],['Default'])]);

infolog/inc/class.infolog_bo.inc.php

@@ -46,6 +46,12 @@ class infolog_bo
 	 * @var boolean
 	 */
 	var $log = false;
+
+	/**
+	 * Access permission cache for current user
+	 */
+	protected static $access_cache = array();
+
 	/**
 	 * Cached timezone data
 	 *
@@ -330,15 +336,13 @@ class infolog_bo
 	 */
 	function check_access($info,$required_rights,$other=0,$user=null)
 	{
-		static $cache = array();
-
 		$info_id = is_array($info) ? $info['info_id'] : $info;
 
 		if (!$user) $user = $this->user;
 		if ($user == $this->user)
 		{
 			$grants = $this->grants;
-			if ($info_id) $access =& $cache[$info_id][$required_rights];	// we only cache the current user!
+			if ($info_id) $access =& static::$access_cache[$info_id][$required_rights];	// we only cache the current user!
 		}
 		else
 		{
@@ -410,6 +414,7 @@ class infolog_bo
 	 */
 	function init()
 	{
+		static::$access_cache = array();
 		$this->so->init();
 	}