From deft-commits-return-304-apmail-incubator-deft-commits-archive=incubator.apache.org@incubator.apache.org Sun Sep 11 18:35:23 2011 Return-Path: X-Original-To: apmail-incubator-deft-commits-archive@minotaur.apache.org Delivered-To: apmail-incubator-deft-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D14EA811D for ; Sun, 11 Sep 2011 18:35:23 +0000 (UTC) Received: (qmail 61777 invoked by uid 500); 11 Sep 2011 18:35:23 -0000 Delivered-To: apmail-incubator-deft-commits-archive@incubator.apache.org Received: (qmail 61761 invoked by uid 500); 11 Sep 2011 18:35:23 -0000 Mailing-List: contact deft-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: deft-dev@incubator.apache.org Delivered-To: mailing list deft-commits@incubator.apache.org Received: (qmail 61752 invoked by uid 99); 11 Sep 2011 18:35:23 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 11 Sep 2011 18:35:23 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 11 Sep 2011 18:35:19 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id D82B523889E3; Sun, 11 Sep 2011 18:34:57 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: svn commit: r1169494 - in /incubator/deft/sandbox: pom.xml src/main/java/org/apache/deft/web/http/HttpProtocol.java src/main/java/org/apache/deft/web/http/HttpResponseImpl.java src/test/java/org/apache/deft/web/http/HttpProtocolTest.java Date: Sun, 11 Sep 2011 18:34:57 -0000 To: deft-commits@incubator.apache.org From: slemesle@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20110911183457.D82B523889E3@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: slemesle Date: Sun Sep 11 18:34:57 2011 New Revision: 1169494 URL: http://svn.apache.org/viewvc?rev=1169494&view=rev Log: - Fix for DEFT-98 - Fix for DEFT-185 - Use properties to map artifacts versions number - Add PowerMock to handle static methods mocking Added: incubator/deft/sandbox/src/test/java/org/apache/deft/web/http/HttpProtocolTest.java Modified: incubator/deft/sandbox/pom.xml incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpProtocol.java incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpResponseImpl.java Modified: incubator/deft/sandbox/pom.xml URL: http://svn.apache.org/viewvc/incubator/deft/sandbox/pom.xml?rev=1169494&r1=1169493&r2=1169494&view=diff ============================================================================== --- incubator/deft/sandbox/pom.xml (original) +++ incubator/deft/sandbox/pom.xml Sun Sep 11 18:34:57 2011 @@ -26,17 +26,28 @@ http://incubator.apache.org/deft 2010 + + r08 + 4.8.2 + 1.8.5 + 1.4.10 + 0.9.28 + 4.1 + 1.6.1 + 1.1.1 + + com.google.guava guava - r08 + ${guava.version} junit junit - 4.8.2 + ${junit.version} jar test @@ -44,35 +55,50 @@ org.mockito mockito-core - 1.8.5 + ${mockito.version} test + + + org.powermock + powermock-module-junit4 + ${powermock.version} + test + + + + org.powermock + powermock-api-mockito + ${powermock.version} + test + + ch.qos.logback logback-classic - 0.9.28 + ${logback.version} jar org.apache.httpcomponents httpclient - 4.1 + ${httpclient.version} test com.ning async-http-client - 1.6.1 + ${ning.version} test javax.activation activation - 1.1.1 + ${activation.version} @@ -225,6 +251,13 @@ Committer + + slemesle + Séven Le Mesle + + Committer + + Modified: incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpProtocol.java URL: http://svn.apache.org/viewvc/incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpProtocol.java?rev=1169494&r1=1169493&r2=1169494&view=diff ============================================================================== --- incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpProtocol.java (original) +++ incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpProtocol.java Sun Sep 11 18:34:57 2011 @@ -22,14 +22,11 @@ package org.apache.deft.web.http; import static org.apache.deft.web.http.HttpServerDescriptor.KEEP_ALIVE_TIMEOUT; import static org.apache.deft.web.http.HttpServerDescriptor.READ_BUFFER_SIZE; +import java.io.FileInputStream; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.MappedByteBuffer; -import java.nio.channels.ClosedChannelException; -import java.nio.channels.SelectableChannel; -import java.nio.channels.SelectionKey; -import java.nio.channels.ServerSocketChannel; -import java.nio.channels.SocketChannel; +import java.nio.channels.*; import java.util.Map; import org.apache.deft.io.IOHandler; @@ -86,21 +83,26 @@ public class HttpProtocol implements IOH SocketChannel clientChannel = (SocketChannel) key.channel(); HttpRequest request = getHttpRequest(key, clientChannel); - if (request.isKeepAlive()) { - ioLoop.addKeepAliveTimeout(clientChannel, Timeout.newKeepAliveTimeout(ioLoop, clientChannel, - KEEP_ALIVE_TIMEOUT)); - } + // Request is null when End-of-Stream have been reached + // No need to do more things right now + if(request != null){ + + if (request.isKeepAlive()) { + ioLoop.addKeepAliveTimeout(clientChannel, Timeout.newKeepAliveTimeout(ioLoop, clientChannel, + KEEP_ALIVE_TIMEOUT)); + } - HttpResponse response = new HttpResponseImpl(this, key, request.isKeepAlive()); - response.setCreateETag(application.getConfiguration().shouldCreateETags()); + HttpResponse response = new HttpResponseImpl(this, key, request.isKeepAlive()); + response.setCreateETag(application.getConfiguration().shouldCreateETags()); - RequestHandler rh = application.getHandler(request); - HttpRequestDispatcher.dispatch(rh, request, response); + RequestHandler rh = application.getHandler(request); + HttpRequestDispatcher.dispatch(rh, request, response); - // Only close if not async. In that case its up to RH to close it (+ - // don't close if it's a partial request). - if (!rh.isMethodAsynchronous(request.getMethod()) && !(request instanceof PartialHttpRequest)) { - response.finish(); + // Only close if not async. In that case its up to RH to close it (+ + // don't close if it's a partial request). + if (!rh.isMethodAsynchronous(request.getMethod()) && !(request instanceof PartialHttpRequest)) { + response.finish(); + } } } @@ -109,7 +111,7 @@ public class HttpProtocol implements IOH logger.debug("handle write..."); SocketChannel channel = (SocketChannel) key.channel(); - if (key.attachment() instanceof MappedByteBuffer) { + if (key.attachment() instanceof FileInputStream) { writeMappedByteBuffer(key, channel); } else if (key.attachment() instanceof DynamicByteBuffer) { writeDynamicByteBuffer(key, channel); @@ -121,17 +123,27 @@ public class HttpProtocol implements IOH } private void writeMappedByteBuffer(SelectionKey key, SocketChannel channel) { - MappedByteBuffer mbb = (MappedByteBuffer) key.attachment(); - if (mbb.hasRemaining()) { - try { - channel.write(mbb); - } catch (IOException e) { - logger.error("Failed to send data to client: {}", e.getMessage()); - Closeables.closeQuietly(channel); + FileInputStream fileInputStream = (FileInputStream) key.attachment(); + + try { + long bytesWritten = 0; + FileChannel fileChannel = fileInputStream.getChannel(); + long sizeNeeded = fileChannel.size(); + bytesWritten = fileChannel.position(); + bytesWritten += fileChannel.transferTo(bytesWritten, sizeNeeded - bytesWritten, channel); + + if (bytesWritten < sizeNeeded){ + // Set channel Position to write rest of data from good starting offset + fileChannel.position(bytesWritten); + }else{ + // Only close channel when file is totally transferred to SocketChannel + com.google.common.io.Closeables.closeQuietly(fileInputStream); + closeOrRegisterForRead(key); } - } - if (!mbb.hasRemaining()) { - closeOrRegisterForRead(key); + } catch (IOException e) { + logger.error("Failed to send data to client: {}", e.getMessage()); + com.google.common.io.Closeables.closeQuietly(fileInputStream); + Closeables.closeQuietly(channel); } } @@ -193,7 +205,8 @@ public class HttpProtocol implements IOH private ByteBuffer reuseAttachment(SelectionKey key) { Object o = key.attachment(); ByteBuffer attachment = null; - if (o instanceof MappedByteBuffer) { + if (o instanceof FileInputStream) { + com.google.common.io.Closeables.closeQuietly(((FileInputStream)o)); attachment = ByteBuffer.allocate(READ_BUFFER_SIZE); } else if (o instanceof DynamicByteBuffer) { attachment = ((DynamicByteBuffer) o).getByteBuffer(); @@ -210,14 +223,21 @@ public class HttpProtocol implements IOH private HttpRequest getHttpRequest(SelectionKey key, SocketChannel clientChannel) { ByteBuffer buffer = (ByteBuffer) key.attachment(); + int bytesRead = -1; try { - clientChannel.read(buffer); + bytesRead = clientChannel.read(buffer); } catch (IOException e) { logger.warn("Could not read buffer: {}", e.getMessage()); Closeables.closeQuietly(ioLoop, clientChannel); } buffer.flip(); + if (bytesRead < 0){ + logger.warn("Reaches end-of-stream on clientChannel"); + Closeables.closeQuietly(ioLoop, clientChannel); + return null; + } + return doGetHttpRequest(key, clientChannel, buffer); } Modified: incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpResponseImpl.java URL: http://svn.apache.org/viewvc/incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpResponseImpl.java?rev=1169494&r1=1169493&r2=1169494&view=diff ============================================================================== --- incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpResponseImpl.java (original) +++ incubator/deft/sandbox/src/main/java/org/apache/deft/web/http/HttpResponseImpl.java Sun Sep 11 18:34:57 2011 @@ -21,9 +21,7 @@ package org.apache.deft.web.http; import static org.apache.deft.web.http.HttpServerDescriptor.WRITE_BUFFER_SIZE; -import java.io.File; -import java.io.IOException; -import java.io.RandomAccessFile; +import java.io.*; import java.nio.MappedByteBuffer; import java.nio.channels.ClosedChannelException; import java.nio.channels.FileChannel; @@ -47,6 +45,7 @@ import com.google.common.base.CharMatche import com.google.common.base.Charsets; import com.google.common.base.Strings; import com.google.common.collect.Maps; +import sun.nio.ch.FileChannelImpl; public class HttpResponseImpl implements HttpResponse { @@ -286,39 +285,27 @@ public class HttpResponseImpl implements long bytesWritten = 0; flush(); // write initial line + headers - RandomAccessFile raf = null; + FileInputStream in = null; try { - raf = new RandomAccessFile(file, "r"); - FileChannel fc = raf.getChannel(); - MappedByteBuffer mbb = raf.getChannel().map(MapMode.READ_ONLY, 0L, fc.size()); - - if (mbb.hasRemaining()) { - bytesWritten = ((SocketChannel) key.channel()).write(mbb); - logger.debug("sent file, bytes sent: {}", bytesWritten); - } - if (mbb.hasRemaining()) { - try { - key.channel().register(key.selector(), SelectionKey.OP_WRITE); // TODO - // RS - // 110621, - // use - // IOLoop.updateHandler - } catch (ClosedChannelException e) { - logger.error("ClosedChannelException during write(File): {}", e.getMessage()); - Closeables.closeQuietly(key.channel()); - } - key.attach(mbb); + in = new FileInputStream(file); + FileChannel fileChannel = in.getChannel(); + long sizeNeeded = fileChannel.size(); + bytesWritten = fileChannel.transferTo(0, sizeNeeded, ((SocketChannel) key.channel())); + + if (bytesWritten < sizeNeeded){ + // Set channel Position to write rest of data from good starting offset + fileChannel.position(bytesWritten); + key.attach(in); + }else{ + // Only close channel when file is totally transferred to SocketChannel + com.google.common.io.Closeables.closeQuietly(in); } + } catch (IOException e) { - logger.error("Error writing (static file) response: {}", e.getMessage()); - } finally { - if (raf != null) { - try { - raf.close(); - } catch (IOException e) { - logger.error("Error closing static file: ", e.getMessage()); - } - } + logger.error("Error writing (static file {}) to response: {}", file.getAbsolutePath(), e.getMessage()); + + // If an exception occurs here we should ensure that file is closed + com.google.common.io.Closeables.closeQuietly(in); } return bytesWritten; Added: incubator/deft/sandbox/src/test/java/org/apache/deft/web/http/HttpProtocolTest.java URL: http://svn.apache.org/viewvc/incubator/deft/sandbox/src/test/java/org/apache/deft/web/http/HttpProtocolTest.java?rev=1169494&view=auto ============================================================================== --- incubator/deft/sandbox/src/test/java/org/apache/deft/web/http/HttpProtocolTest.java (added) +++ incubator/deft/sandbox/src/test/java/org/apache/deft/web/http/HttpProtocolTest.java Sun Sep 11 18:34:57 2011 @@ -0,0 +1,136 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.deft.web.http; + +import com.google.common.collect.Maps; +import org.apache.deft.io.IOLoop; +import org.apache.deft.util.Closeables; +import org.apache.deft.web.Application; +import org.apache.deft.web.handler.RequestHandler; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import java.nio.ByteBuffer; +import java.nio.channels.SelectableChannel; +import java.nio.channels.SelectionKey; +import java.nio.channels.Selector; +import java.nio.channels.SocketChannel; + +/** + * + * User: slemesle + * Date: 11/09/11 + * Time: 12:35 + */ +@RunWith(PowerMockRunner.class) +@PrepareForTest(value = {Closeables.class}) +@PowerMockIgnore("javax.management.*") +public class HttpProtocolTest { + + private IOLoop ioLoop; + private HttpProtocol protocol; + private SelectionKey key; + private SocketChannel socketChannel; + + + @Before + public void beforeTests(){ + + ioLoop = Mockito.mock(IOLoop.class); + socketChannel = Mockito.mock(SocketChannel.class); + key = new MySelectionKey(socketChannel); + PowerMockito.mockStatic(Closeables.class); + protocol = new HttpProtocol(ioLoop, new Application(Maps.newHashMap())); + } + + @Test + public void testHandleReadReachEOF() throws Exception { + + ByteBuffer byteBuffer = ByteBuffer.allocate(10); + key.attach(byteBuffer); + + // See what happens when read returns -1 + Mockito.when(socketChannel.read(byteBuffer)).thenReturn(-1); + + protocol.handleRead(key); + + Mockito.verify(socketChannel).read(byteBuffer); + + // CloseQuietly should have been called for this channel EOF + PowerMockito.verifyStatic(Mockito.times(1)); + Closeables.closeQuietly(ioLoop, socketChannel); + } + + + /** + * Since did not succeed in mocking final fields + * here is a short mock for the SelectionKey + */ + class MySelectionKey extends SelectionKey { + + SelectableChannel channel; + + MySelectionKey(SelectableChannel channel) { + super(); + this.channel = channel; + } + + @Override + public SelectableChannel channel() { + return channel; + } + + @Override + public Selector selector() { + return null; + } + + @Override + public boolean isValid() { + return false; + } + + @Override + public void cancel() { + // Nothing Todo + } + + @Override + public int interestOps() { + return 0; + } + + @Override + public SelectionKey interestOps(int i) { + return this; + } + + @Override + public int readyOps() { + return 0; + } + } +}