Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with gl_push_file related to file_path #73

Closed
mpolano opened this issue Aug 25, 2022 · 4 comments
Closed

Issue with gl_push_file related to file_path #73

mpolano opened this issue Aug 25, 2022 · 4 comments
Labels

Comments

@mpolano
Copy link
Contributor

mpolano commented Aug 25, 2022

I cannot succeed in using gl_push_file() with an existing file like "folder/file.txt".

The problem is that function gl_file_exists() uses the plain file path while, to get the file, the call to gitlab() function should use the URL encoded version of file_path "folder%2Ffile.txt".
Consequence is that with an existing file, gl_push_file() do not work neither using the plain path or the encoded path works.

@statnmap
Copy link
Member

I am not sure how this should work for subdirectories.
But pushed at the rroot of the project, this works in the unit tests: https://github.com/statnmap/gitlabr/blob/aca7bbabe4dff21959e34e41c695fb79a83a5866/tests/testthat/test_files.R#L60

However, the function does not send an existing file, It creates a file with the content sent as text. Otherwise, you may want to look at this issue: #69
Maybe the name of the function is misleading though.

Here is the documentation of the API call used, if you would like to look at it: https://docs.gitlab.com/ee/api/repository_files.html#create-new-file-in-repository

And yes, the path should be URL encoded

I'd be happy to receive a Pull request for a fix.

@mpolano
Copy link
Contributor Author

mpolano commented Aug 25, 2022

I am willing to pull a request to fix but I just want to agree on how to manage this.
As you say, the path should be URL encoded.
The problem is that gl_file_exists() expects a plain path (not URL encoded) because it uses gl_list_files and matches on its results ( https://docs.gitlab.com/ee/api/repositories.html#list-repository-tree).

I see two options:

  1. Change gl_file_exists() so that it takes an URL encoded file_path
  2. Change gl_push_file() so that is decodes file_path before calling gl_file_exists()

I would prefer option 1 for consistency with other function calls but I understand that, to avoid problems with existing code, option 2 is better. What do you think ?

@statnmap
Copy link
Member

statnmap commented Aug 25, 2022

I think that parameters should be user-friendly. Then, option 2.
Hence,

  • gl_file_exists() needs to take a classical file_path as input.
  • gl_push_file() also take a classical file_path, but is URL encoded internally
  • This also needs to be consistent with gl_get_file() and gl_delete_file(), which do also need a URL encoded path

I think you only need to add URLencode("foo/bar", reserved = TRUE) to each of these three functions.
I'd like if you can come with a unit test for each of them to add to the existing ones: push, get, then delete the file.

Following CONTRIBUTING may be long. I would accept an example with your own repository, that I can adapt to the {gitlabr} unit tests.

@statnmap
Copy link
Member

This is now included in the development version of gitlabr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants