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

Feature/range send file #1115

Merged
merged 5 commits into from Aug 19, 2015
Merged

Feature/range send file #1115

merged 5 commits into from Aug 19, 2015

Conversation

pmlopes
Copy link
Contributor

@pmlopes pmlopes commented Aug 6, 2015

Implement range support for streaming files.
Once this is in we can support:

  • range aware static file handling on web
  • resumable downloads
  • send chunks of files using netclients
  • etc...

@@ -439,7 +439,7 @@ private void doSendFile(String filename, Handler<AsyncResult<Void>> resultHandle
try {
raf = new RandomAccessFile(file, "r");
conn.queueForWrite(response);
conn.sendFile(raf, fileLength);
conn.sendFile(raf, Math.min(offset, fileLength), Math.min(length, fileLength - offset));
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicate calculation. can be done once higher up in method.

…mple on web servers to support range requests, resumable downloads, etc...

Signed-off-by: Paulo Lopes <paulo@mlopes.net>
(cherry picked from commit bcb6cb0)
Signed-off-by: Paulo Lopes <paulo@mlopes.net>
(cherry picked from commit 4e68c8e)
@pmlopes
Copy link
Contributor Author

pmlopes commented Aug 17, 2015

i've ammended the branch...

@@ -230,6 +230,36 @@ public void testSendFileFromClasspath() {
await();
}

@Test
public void testSendOpenRangeFileFromClasspath() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more appropriate to have these tests in HttpTest with the rest of the send file tests.

We do have some send file tests in FileResolverTest but these are just to make sure the offsetting is working ok.

Signed-off-by: Paulo Lopes <paulo@mlopes.net>
Signed-off-by: Paulo Lopes <paulo@mlopes.net>
}

/**
* Ask the OS to stream a file as specified by {@code filename} directly
Copy link
Contributor

Choose a reason for hiding this comment

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

When creating an overloaded method, instead of duplicating the javadoc, it's preferable to use {@link} to refer to the version with the full javadoc and just explain the differences.

@purplefox
Copy link
Contributor

Also, we should probably have something in the docs (package-info.java in the http package) about this.

Signed-off-by: Paulo Lopes <paulo@mlopes.net>
purplefox added a commit that referenced this pull request Aug 19, 2015
@purplefox purplefox merged commit cf11d8f into eclipse-vertx:master Aug 19, 2015
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.

None yet

2 participants