You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by solomax <gi...@git.apache.org> on 2017/04/12 08:44:54 UTC
[GitHub] wicket pull request #219: [WICKET-6355] It is now possible to set fileName t...
GitHub user solomax opened a pull request:
https://github.com/apache/wicket/pull/219
[WICKET-6355] It is now possible to set fileName to FileSystemResource
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/solomax/wicket master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/219.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #219
----
commit 71d5be39df87875d3550a05f44df8ba3e6659036
Author: Maxim Solodovnik <so...@gmail.com>
Date: 2017-04-12T08:44:16Z
[WICKET-6355] It is now possible to set fileName to FileSystemResource
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #219: [WICKET-6355] It is now possible to set fileName t...
Posted by solomax <gi...@git.apache.org>.
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/219#discussion_r111130572
--- Diff: wicket-core/src/main/java/org/apache/wicket/resource/FileSystemResource.java ---
@@ -95,7 +95,9 @@ protected ResourceResponse createResourceResponse(Path path, String fileName)
resourceResponse.setContentType(getMimeType());
resourceResponse.setAcceptRange(ContentRangeType.BYTES);
resourceResponse.setContentLength(size);
- resourceResponse.setFileName(fileName == null ? path.getFileName().toString() : fileName);
+ if (path != null && path.getFileName() != null) {
--- End diff --
My bad, will fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #219: [WICKET-6355] It is now possible to set fileName t...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/wicket/pull/219
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #219: [WICKET-6355] It is now possible to set fileName t...
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/219#discussion_r111130176
--- Diff: wicket-core/src/main/java/org/apache/wicket/resource/FileSystemResource.java ---
@@ -95,7 +95,9 @@ protected ResourceResponse createResourceResponse(Path path, String fileName)
resourceResponse.setContentType(getMimeType());
resourceResponse.setAcceptRange(ContentRangeType.BYTES);
resourceResponse.setContentLength(size);
- resourceResponse.setFileName(fileName == null ? path.getFileName().toString() : fileName);
+ if (path != null && path.getFileName() != null) {
--- End diff --
I think we don't need to check *path != null* because it is checked before (and a WicketRuntimeException is thrown if it is null) - everything else looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #219: [WICKET-6355] It is now possible to set fileName t...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/219#discussion_r111096456
--- Diff: wicket-core/src/main/java/org/apache/wicket/resource/FileSystemResource.java ---
@@ -68,17 +68,19 @@ public FileSystemResource()
@Override
protected ResourceResponse newResourceResponse(Attributes attributes)
{
- return createResourceResponse(path);
+ return createResourceResponse(path, null);
}
/**
* Creates a resource response based on the given attributes
*
* @param path
* the path to create the resource response with
+ * @param fileName
+ * fileName to set, path.getFileName() will be used in case null passed
* @return the actual resource response x
*/
- protected ResourceResponse createResourceResponse(Path path)
+ protected ResourceResponse createResourceResponse(Path path, String fileName)
--- End diff --
Do we really need this extra parameter here for the fileName ?
IMO by default it should do: `if (fileName != null) resourceResponse.setFileName(path.getFileName().toString());`
If the application wants to override it then do something like:
```java
ResourceResponse rr = super.createResourceResponse(path);
rr.setFileName("custom.name");
return rr;
```
Also I think passing `Attributes` as a parameter would be useful!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---