Skip to content

Conversation

@PhilippeVienne
Copy link
Contributor

No description provided.

@gmessner
Copy link
Collaborator

@PhilippeVienne
Thanks for the PR, weird that GitLab excludes the Content-Disposition header. That being said, I would prefer that a parameter is added for the filename, this would allow the user to control the name of the file (we already give them an option of setting the directory).

public File downloadExport(Object projectIdOrPath, File directory, String filename)

The current method downloadExport(Object projectIdOrPath, File directory) would simply call:

return (downloadExport(projectIdOrPath, directory, null);

In downloadExport(Object projectIdOrPath, File directory, String filename) if both the user provided filename and the Content-Disposition header are null, then a filename would need to be constructed. I would also like to make the constructed filename a bit more useful and patterned after the filename that is returned by non gitlab.com servers. That filename looks like this:
2019-06-08_03-46-178_gitlab4j_test-project_export.tar.gz

My suggestion for constructing the filename is to use a format:

projectIdOrPath is an Integer (project ID)::

String template = "%1$tY-%1$tm-%1$td_%1$tH-%1$tM-%1%tS_projectid-%2$d_export.tar.gz";
String filename = template.format(neew Date(), projectIdOrPath);
// filename = "2019-06-10_10-28-52_projectid_3115610_export.tar.gz"

projectIdOrPath is a String instance, use it:

String template = "%1$tY-%1$tm-%1$td_%1$tH-%1$tM-%1$tS_%2$s_export.tar.gz";
String filename = String.format(template, new Date(), projectIdOrPath);
// filename = "2019-06-10_10-28-52_test-project_export.tar.gz"

projectIdOrPath is a Project instance, use the project path:

String template = "%1$tY-%1$tm-%1$td_%1$tH-%1$tM-%1$tS_%2$s_export.tar.gz";
String filename = String.format(template, new Date(), testProject.getPath());
// filename = "2019-06-10_10-28-52_test-project_export.tar.gz"

@gmessner
Copy link
Collaborator

It might be better to use getPathWithNamespace() for the project instance and make sure to replace any slashes / with an underscore _

@PhilippeVienne
Copy link
Contributor Author

Working on requested changes 😃

@gmessner
Copy link
Collaborator

@PhilippeVienne
Once again, thanks for the contribution. Just one small request above, everything else looks great.

Also, would be good to add integration tests for these other conditions.

@gmessner gmessner merged commit 2228180 into gitlab4j:master Jun 10, 2019
@PhilippeVienne PhilippeVienne deleted the fix/null-on-content-disposition-download branch June 11, 2019 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants