Skip to content

Commit

Permalink
Change the requirement for deleting entries from ScriptRepository
Browse files Browse the repository at this point in the history
Request "Get rid of the local file deletion. I don't think users will expect this, make delete only delete from the repository."

The script repository will not remove files locally. The ScriptRepository.h
indicates this change and ScriptRepositoryTestImpl.h translate the current
requirement on removing entries from repository.

re #7031
  • Loading branch information
gesnerpassos committed Jun 4, 2013
1 parent 45b3387 commit 3a1d335
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 45 deletions.
13 changes: 2 additions & 11 deletions Code/Mantid/Framework/API/inc/MantidAPI/ScriptRepository.h
Original file line number Diff line number Diff line change
Expand Up @@ -491,13 +491,13 @@ They will work as was expected for folders @ref folders-sec.
const std::string & email) = 0;

/**
Delete the file from the local folder and from the remote repository.
Delete the file from the remote repository (it does not touch the local copy).
After this, the file will not be available for anyone among the users. As so,
it is required a justification of why to remove (the comment), and the current
rule accept that only the owner of the file (which is considered the last one
to use the file) is allowed to remove it.
The file must be removed from the central repository (git)
The file will be removed from the central repository (git)
@note This operation requires internet connection.
Expand All @@ -517,15 +517,6 @@ They will work as was expected for folders @ref folders-sec.
virtual void remove(const std::string & file_path, const std::string & comment,
const std::string & author,
const std::string & email) = 0;
/** Delete the file from the local repository only. It does not touch the central repository.
This operation does not require internet connection. Although it is not strictly necessary
to remove files using this method remove_local, because, the ScriptRepository does recognize
files being removed from the folder, it may be used for convenience.
@param file_path for the file to be deleted.
*/
virtual void remove_local(const std::string & file_path) = 0;

/** Define the file patterns that will not be listed in listFiles.
This is important to force the ScriptRepository to not list hidden files,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -827,9 +827,8 @@ void test_downloading_locally_modified_file(){
}

/**This test ensure that when you remove a file from the central repository,
the remove method,
the entry can not be listed anymore, internally or externally
the entry will be available only internally as LOCAL_ONLY file.
*/
void test_delete_remove_valid_file_from_central_repository(){
TS_ASSERT_THROWS_NOTHING(repo->install(local_rep));
Expand All @@ -845,17 +844,17 @@ void test_downloading_locally_modified_file(){
TS_ASSERT_THROWS_NOTHING(repo->remove(file_name, "please remove it","noauthor","noemail"));

// you should not find the file, so fileStatus should throw exception entry not inside repository
TS_ASSERT_THROWS(repo->fileStatus(file_name),ScriptRepoException);
TS_ASSERT(repo->fileStatus(file_name) == Mantid::API::LOCAL_ONLY);

// even if you re-read the repository listing the files
TS_ASSERT_THROWS_NOTHING(repo->listFiles());

// you should not find this file agin
TS_ASSERT_THROWS(repo->fileStatus(file_name),ScriptRepoException);
TS_ASSERT(repo->fileStatus(file_name) == Mantid::API::LOCAL_ONLY);

// assert file does not exists inside the local folder
// assert file does exist inside the local folder
Poco::File f(std::string(local_rep).append(file_name));
TS_ASSERT(!f.exists());
TS_ASSERT(f.exists());
}


Expand Down Expand Up @@ -885,31 +884,6 @@ void test_delete_remove_valid_file_from_central_repository_simulate_server_rejec
TS_ASSERT(repo->fileStatus(file_name) == Mantid::API::BOTH_UNCHANGED) ;
}

/**
Test what happens with a file is removed only locally (remove_local).
If the file existed in the central repository, after removing the local copy,
its state must be REMOVE_ONLY
*/
void test_delete_remove_file_locally(){
TS_ASSERT_THROWS_NOTHING(repo->install(local_rep));
TS_ASSERT_THROWS_NOTHING(repo->listFiles());
std::string file_name = "TofConv/TofConverter.py";
// download
TS_ASSERT_THROWS_NOTHING(repo->download(file_name));

// it must be unchanged
TS_ASSERT(repo->fileStatus(file_name) == Mantid::API::BOTH_UNCHANGED) ;

// now, lets delete this file locally
TS_ASSERT_THROWS_NOTHING(repo->remove_local(file_name));

// you should find the file but status REMOTE_ONLY
TS_ASSERT(repo->fileStatus(file_name) == Mantid::API::REMOTE_ONLY) ;
// nothing should change listing all the files again
TS_ASSERT_THROWS_NOTHING(repo->listFiles());
TS_ASSERT(repo->fileStatus(file_name) == Mantid::API::REMOTE_ONLY) ;
}

/** Test invalid entry for removing files, when they are not local (not downloaded)
Ensure that removing from the central repository is not allowed, if the file has
Expand All @@ -919,8 +893,7 @@ void test_delete_remove_valid_file_from_central_repository_simulate_server_rejec
TS_ASSERT_THROWS_NOTHING(repo->install(local_rep));
TS_ASSERT_THROWS_NOTHING(repo->listFiles());
std::string file_name = "TofConv/TofConverter.py";



// attempt to remove file that is not local (no download was done)
// it must throw exception, to inform that it is not allowed to remove it.
TS_ASSERT_THROWS(repo->remove(file_name, "please remove it","noauthor","noemail")
Expand Down

0 comments on commit 3a1d335

Please sign in to comment.