Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Clean repository on package purge #2131

Merged
merged 5 commits into from Apr 14, 2017

Conversation

orthagh
Copy link
Contributor

@orthagh orthagh commented Apr 12, 2017

No description provided.

@orthagh orthagh requested review from trasher and wawax April 12, 2017 07:36
@orthagh orthagh changed the title fix purge and clean of packages Clean repository on package purge Apr 12, 2017
@orthagh
Copy link
Contributor Author

orthagh commented Apr 12, 2017

will do a test for this

@orthagh
Copy link
Contributor Author

orthagh commented Apr 12, 2017

Hum, in fact, i don't see any way to achieve a fileupload with phpunit.
So if no suggestions, i think we can merge as if

@trasher
Copy link
Contributor

trasher commented Apr 13, 2017

As far as I can see in existing code, nothing would prevent to simulate an upload.

The only thing that should prevent fake file upload would be use of is_uploaded_file or 'move_uploaded_file` methods. If those functions are not used, you just have to cal the upload method with required informations you can set from unit test.


//remove file
unset($files[$index]);
//array_splice($datas['jobs']['associatedFiles'], $index, 1);
unset($datas['associatedFiles'][$sha512]);
//array_splice($data['jobs']['associatedFiles'], $index, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this line could be dropped

* @return boolean
*/
static function removeFileInRepo($sha512, $packages_id) {
static function removeFileInRepo($sha512) {
$repoPath = GLPI_PLUGIN_DOC_DIR."/fusioninventory/files/repository/";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not paths be declared elsewhere? I guess same ones are used on the upload part; it would be better to have only one var to change/adapt for both of them.

http://www.gnu.org/licenses/agpl-3.0-standalone.html
@link http://www.fusioninventory.org/
@link http://forge.fusioninventory.org/projects/fusioninventory-for-glpi/
@since 2013
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad year :p

------------------------------------------------------------------------
*/

class RepositoryTest extends RestoreDatabase_TestCase {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I've understand well the above code, it is possible to have directories that will be dropped if there is no longer any file into. Maybe add a tests case for that?

@orthagh orthagh merged commit 20f590c into fusioninventory:glpi9.1 Apr 14, 2017
orthagh added a commit that referenced this pull request Apr 14, 2017
* Clean repository on package purge

* add tests

* unlink file in tests

* fix pr comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants