Reuse OC10 resource (code) as much as possible for OCIS

[SagarGi]

Description

Recently we have been shifting some spaces related tests from core to ocis. During the process we have been duplicating code on ocis which is already in core. So we need to mitigate the code duplication and use the already available code (already in core) for new implementation on ocis.

Note: Follow up this PR for reduction of code duplication.
owncloud/core#40328
#4492

NOTE:

Make possible code reduction related to the features that has been added during this issue:
#4154 (comment)

[SagarGi]

TODO:
(Keep on adding features where you find duplicate code)

• lockspaces.feature (@SagarGi ) [tests-only][full-ci]Reuse locking method from core #4510
• favourites.feature (@SagarGi ) [full-ci][tests-only]Refactor and Reuse code from core #4535
• checksum.feature (@SagarGi ) [tests-only][full-ci]Reuse code from core #4536
• shareOperation.feature (@SagarGi ) [tests-only][full-ci]Reuse code from core for shareOperation #4538

[amrita-shrestha]

@SagarGi can't we use core code for this also?

ocis/tests/acceptance/features/bootstrap/SpacesContext.php

Lines 520 to 618 in b0fa917

	/**
	* Send Propfind Request to Url
	*
	* @param string $fullUrl
	* @param string $user
	* @param string $password
	* @param string $xRequestId
	* @param array $headers
	* @param mixed $body
	*
	* @return ResponseInterface
	*
	* @throws GuzzleException
	*/
	public function sendPropfindRequestToUrl(
	string $fullUrl,
	string $user,
	string $password,
	string $xRequestId = '',
	array $headers = [],
	$body = null
	): ResponseInterface {
	return HttpRequestHelper::sendRequest($fullUrl, $xRequestId, 'PROPFIND', $user, $password, $headers, $body);
	}
	/**
	* Send Put Request to Url
	*
	* @param string $fullUrl
	* @param string $user
	* @param string $password
	* @param string $xRequestId
	* @param array $headers
	* @param string $content
	*
	* @return ResponseInterface
	*
	* @throws GuzzleException
	*/
	public function sendPutRequestToUrl(
	string $fullUrl,
	string $user,
	string $password,
	string $xRequestId = '',
	array $headers = [],
	string $content = ""
	): ResponseInterface {
	return HttpRequestHelper::sendRequest($fullUrl, $xRequestId, 'PUT', $user, $password, $headers, $content);
	}
	/**
	* Send POST Request to url
	*
	* @param string $fullUrl
	* @param string $user
	* @param string $password
	* @param mixed $body
	* @param string $xRequestId
	* @param array $headers
	*
	* @return ResponseInterface
	*
	* @throws GuzzleException
	*/
	public function sendPostRequestToUrl(
	string $fullUrl,
	string $user,
	string $password,
	$body,
	string $xRequestId = '',
	array $headers = []
	): ResponseInterface {
	return HttpRequestHelper::post($fullUrl, $xRequestId, $user, $password, $headers, $body);
	}
	/**
	* Send Graph Create Folder Request
	*
	* @param string $fullUrl
	* @param string $method
	* @param string $user
	* @param string $password
	* @param string $xRequestId
	* @param array $headers
	*
	* @return ResponseInterface
	*
	* @throws GuzzleException
	*/
	public function sendCreateFolderRequest(
	string $fullUrl,
	string $method,
	string $user,
	string $password,
	string $xRequestId = '',
	array $headers = []
	): ResponseInterface {
	return HttpRequestHelper::sendRequest($fullUrl, $xRequestId, $method, $user, $password, $headers);
	}

[SagarGi]

@amrita-shrestha i think we can but i think we cannot say its a duplicate code.

[amrita-shrestha]

@amrita-shrestha i think we can but i think we cannot say its a duplicate code.

yeah but if we are reusing core code then why not properly implement it in overall ocis code?
I think it will be nice if you look over all ocis codes and check if there was pre-existing core code in ocis.
This way we can provide consistency in ocis code.

[SwikritiT]

@amrita-shrestha i think we can but i think we cannot say its a duplicate code.

yeah but if we are reusing core code then why not properly implement it in overall ocis code? I think it will be nice if you look over all ocis codes and check if there was pre-existing core code in ocis. This way we can provide consistency in ocis code.

I'll take a look what can be done for this

[SwikritiT]

I think I've reused as many tests as possible in owncloud/core#40371 and #4577 but if there are still other tests that can be reused that I might have left please take a look and mention here

@SagarGi @phil-davis @amrita-shrestha

[SagarGi]

@SwikritiT can we also reuse for copy and move request? for (copySpaces.feature and moveSpaces.feature).

[SwikritiT]

@SwikritiT can we also reuse for copy and move request? for (copySpaces.feature and moveSpaces.feature).

I'll take a look

[amrita-shrestha]

ocis/tests/acceptance/features/apiSpaces/changingFilesShare.feature

Line 17 in 28c6311

And user "Alice" has moved file "textfile0.txt" to "PARENT/from_alice.txt" in space "Personal"

ocis/tests/acceptance/features/bootstrap/SpacesContext.php

Lines 1590 to 1648 in 28c6311

	/**
	* @When /^user "([^"]*)" moves (?:file|folder) "([^"]*)" to "([^"]*)" in space "([^"]*)" using the WebDAV API$/
	*
	* @param string $user
	* @param string $fileSource
	* @param string $fileDestination
	* @param string $spaceName
	*
	* @return void
	* @throws GuzzleException
	*/
	public function userMovesFileWithinSpaceUsingTheWebDAVAPI(
	string $user,
	string $fileSource,
	string $fileDestination,
	string $spaceName
	):void {
	$space = $this->getSpaceByName($user, $spaceName);
	$headers['Destination'] = $this->destinationHeaderValueWithSpaceName(
	$user,
	$fileDestination,
	$spaceName
	);
	$fileSource = $this->escapePath(\trim($fileSource, "/"));
	$fullUrl = $space["root"]["webDavUrl"] . '/' . $fileSource;
	$this->moveFilesAndFoldersRequest($user, $fullUrl, $headers);
	}
	/**
	* @Given /^user "([^"]*)" has moved (?:file|folder) "([^"]*)" to "([^"]*)" in space "([^"]*)"$/
	*
	* @param string $user
	* @param string $fileSource
	* @param string $fileDestination
	* @param string $spaceName
	*
	* @return void
	* @throws GuzzleException
	*/
	public function userHasMovedFileWithinSpaceUsingTheWebDAVAPI(
	string $user,
	string $fileSource,
	string $fileDestination,
	string $spaceName
	):void {
	$this->userMovesFileWithinSpaceUsingTheWebDAVAPI(
	$user,
	$fileSource,
	$fileDestination,
	$spaceName
	);
	$this->featureContext->theHTTPStatusCodeShouldBe(
	201,
	__METHOD__ . "Expected response status code should be 201 (Created)\n" .
	"Actual response status code was: " . $this->featureContext->getResponse()->getStatusCode() . "\n" .
	"Actual response body was: " . $this->featureContext->getResponse()->getBody()
	);
	}

This step contain some core code maybe we can remove that too

[SwikritiT]

ocis/tests/acceptance/features/apiSpaces/changingFilesShare.feature

Line 17 in 28c6311

And user "Alice" has moved file "textfile0.txt" to "PARENT/from_alice.txt" in space "Personal"

ocis/tests/acceptance/features/bootstrap/SpacesContext.php

Lines 1590 to 1648 in 28c6311

	/**
	* @When /^user "([^"]*)" moves (?:file|folder) "([^"]*)" to "([^"]*)" in space "([^"]*)" using the WebDAV API$/
	*
	* @param string $user
	* @param string $fileSource
	* @param string $fileDestination
	* @param string $spaceName
	*
	* @return void
	* @throws GuzzleException
	*/
	public function userMovesFileWithinSpaceUsingTheWebDAVAPI(
	string $user,
	string $fileSource,
	string $fileDestination,
	string $spaceName
	):void {
	$space = $this->getSpaceByName($user, $spaceName);
	$headers['Destination'] = $this->destinationHeaderValueWithSpaceName(
	$user,
	$fileDestination,
	$spaceName
	);
	$fileSource = $this->escapePath(\trim($fileSource, "/"));
	$fullUrl = $space["root"]["webDavUrl"] . '/' . $fileSource;
	$this->moveFilesAndFoldersRequest($user, $fullUrl, $headers);
	}
	/**
	* @Given /^user "([^"]*)" has moved (?:file|folder) "([^"]*)" to "([^"]*)" in space "([^"]*)"$/
	*
	* @param string $user
	* @param string $fileSource
	* @param string $fileDestination
	* @param string $spaceName
	*
	* @return void
	* @throws GuzzleException
	*/
	public function userHasMovedFileWithinSpaceUsingTheWebDAVAPI(
	string $user,
	string $fileSource,
	string $fileDestination,
	string $spaceName
	):void {
	$this->userMovesFileWithinSpaceUsingTheWebDAVAPI(
	$user,
	$fileSource,
	$fileDestination,
	$spaceName
	);
	$this->featureContext->theHTTPStatusCodeShouldBe(
	201,
	__METHOD__ . "Expected response status code should be 201 (Created)\n" .
	"Actual response status code was: " . $this->featureContext->getResponse()->getStatusCode() . "\n" .
	"Actual response body was: " . $this->featureContext->getResponse()->getBody()
	);
	}

This step contain some core code maybe we can remove that too

We can reuse the code from the core for within space move or copy but for move in-between space, it's not possible through oc10 code so I don't think we should refactor these

cc @amrita-shrestha @SagarGi

[SagarGi]

@SwikritiT can you remove this method:

/**
	 * Send Propfind Request to Url
	 *
	 * @param  string $fullUrl
	 * @param  string $user
	 * @param  string $password
	 * @param  string $xRequestId
	 * @param  array  $headers
	 * @param mixed $body
	 *
	 * @return ResponseInterface
	 *
	 * @throws GuzzleException
	 */
	public function sendPropfindRequestToUrl(
		string $fullUrl,
		string $user,
		string $password,
		string $xRequestId = '',
		array $headers = [],
		$body = null
	): ResponseInterface {
		return HttpRequestHelper::sendRequest($fullUrl, $xRequestId, 'PROPFIND', $user, $password, $headers, $body);
	}

and make a direct request in the method:

userListAllDeletedFilesInTrash```
May be we dont require the propfind method anymore since we are using it from `core`.

[SwikritiT]

@SwikritiT can you remove this method:

/**
	 * Send Propfind Request to Url
	 *
	 * @param  string $fullUrl
	 * @param  string $user
	 * @param  string $password
	 * @param  string $xRequestId
	 * @param  array  $headers
	 * @param mixed $body
	 *
	 * @return ResponseInterface
	 *
	 * @throws GuzzleException
	 */
	public function sendPropfindRequestToUrl(
		string $fullUrl,
		string $user,
		string $password,
		string $xRequestId = '',
		array $headers = [],
		$body = null
	): ResponseInterface {
		return HttpRequestHelper::sendRequest($fullUrl, $xRequestId, 'PROPFIND', $user, $password, $headers, $body);
	}

and make a direct request in the method:

userListAllDeletedFilesInTrash```
May be we dont require the propfind method anymore since we are using it from `core`.

Done in #4710

[SagarGi]

@SwikritiT I have scanned as much possible. I think we have reused lots of code from core. Can't really find out possible code to be reused. So may be this issue can be closed. @saw-jan @amrita-shrestha

[phil-davis]

I merged #4710 so this issue is done.