You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2007/07/08 15:40:21 UTC
svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk: ./
module-main/src/main/java/org/apache/http/protocol/
module-main/src/test/java/org/apache/http/protocol/
Author: olegk
Date: Sun Jul 8 06:40:19 2007
New Revision: 554367
URL: http://svn.apache.org/viewvc?view=rev&rev=554367
Log:
HTTPCORE-88: Add convenience methods to HttpRequestInterceptorList, HttpResponseInterceptorList
Contributed by Andrea Selva <selva.andre at gmail.com>
Reviewed by Oleg Kalnichevski
Added:
jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java (with props)
Modified:
jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/BasicHttpProcessor.java
jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpRequestInterceptorList.java
jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpResponseInterceptorList.java
jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestAllProtocol.java
Modified: jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt?view=diff&rev=554367&r1=554366&r2=554367
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt (original)
+++ jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt Sun Jul 8 06:40:19 2007
@@ -1,5 +1,9 @@
Changes since release 4.0 Alpha 5
+* [HTTPCORE-88] Added convenience methods to HttpRequestInterceptorList,
+ HttpResponseInterceptorList interfaces
+ Contributed by Andrea Selva <selva.andre at gmail.com>
+
* [HTTPCORE-89]: Fixed bug in DefaultConnectingIOReactor causing incorrect
handling of local (immediate) connections on some platforms (affects Sun
Solaris 2.9 / Sparc and likely other Solaris 2.x platforms)
Modified: jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/BasicHttpProcessor.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/BasicHttpProcessor.java?view=diff&rev=554367&r1=554366&r2=554367
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/BasicHttpProcessor.java (original)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/BasicHttpProcessor.java Sun Jul 8 06:40:19 2007
@@ -33,6 +33,7 @@
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
import org.apache.http.HttpException;
@@ -45,6 +46,7 @@
* Keeps lists of interceptors for processing requests and responses.
*
* @author <a href="mailto:oleg at ural.ru">Oleg Kalnichevski</a>
+ * @author Andrea Selva
*
* @version $Revision$
*
@@ -68,43 +70,112 @@
}
this.requestInterceptors.add(itcp);
}
+
+ // non-Javadoc, see interface HttpRequestInterceptorList
+ public void addRequestInterceptor(final HttpRequestInterceptor itcp, int index) {
+
+ if (itcp == null) {
+ return;
+ }
+ if (this.requestInterceptors == null) {
+ this.requestInterceptors = new ArrayList();
+ }
+
+ if (index == Integer.MAX_VALUE) {
+ // Add last
+ this.requestInterceptors.add(this.requestInterceptors.size() - 1, itcp);
+ } else {
+ // Insert at preferred index
+ this.requestInterceptors.add(index, itcp);
+ }
+ }
+
+ public void addResponseInterceptor(HttpResponseInterceptor itcp, int index) {
+ if (itcp == null) {
+ return;
+ }
+ if (this.responseInterceptors == null) {
+ this.responseInterceptors = new ArrayList();
+ }
+
+ if (index == Integer.MAX_VALUE) {
+ // Add last
+ this.responseInterceptors.add(this.responseInterceptors.size() - 1, itcp);
+ } else {
+ // Insert at preferred index
+ this.responseInterceptors.add(index, itcp);
+ }
+ }
+
+
+ // non-Javadoc, see interface HttpRequestInterceptorList
+ public void removeRequestInterceptorByClass(final Class clazz) {
+ if (this.requestInterceptors == null) {
+ return;
+ }
+ for (Iterator it = this.requestInterceptors.iterator(); it.hasNext(); ) {
+ Object request = it.next();
+ if (request.getClass().equals(clazz)) {
+ it.remove();
+ }
+ }
+ }
+
+ // non-Javadoc, see interface HttpResponseInterceptorList
+ public void removeResponseInterceptorByClass(final Class clazz) {
+ if (this.responseInterceptors == null) {
+ return;
+ }
+ for (Iterator it = this.responseInterceptors.iterator(); it.hasNext(); ) {
+ Object request = it.next();
+ if (request.getClass().equals(clazz)) {
+ it.remove();
+ }
+ }
+ }
+
/**
* Same as {@link #addRequestInterceptor addRequestInterceptor}.
*
* @param interceptor the interceptor to add
*/
public final
- void addInterceptor(final HttpRequestInterceptor interceptor) {
+ void addInterceptor(final HttpRequestInterceptor interceptor) {
addRequestInterceptor(interceptor);
}
-
-
+
+ public final
+ void addInterceptor(final HttpRequestInterceptor interceptor, int index) {
+ addRequestInterceptor(interceptor, index);
+ }
+
+
// non-Javadoc, see interface HttpRequestInterceptorList
public int getRequestInterceptorCount() {
return (this.requestInterceptors == null) ?
0 : this.requestInterceptors.size();
}
-
-
+
+
// non-Javadoc, see interface HttpRequestInterceptorList
public HttpRequestInterceptor getRequestInterceptor(int index) {
if ((this.requestInterceptors == null) ||
- (index < 0) || (index >= this.requestInterceptors.size()))
+ (index < 0) || (index >= this.requestInterceptors.size()))
return null;
-
+
return (HttpRequestInterceptor) this.requestInterceptors.get(index);
}
-
-
+
+
// non-Javadoc, see interface HttpRequestInterceptorList
public void clearRequestInterceptors() {
this.requestInterceptors = null;
}
-
-
-
+
+
+
// non-Javadoc, see interface HttpResponseInterceptorList
public void addResponseInterceptor(final HttpResponseInterceptor itcp) {
if (itcp == null) {
@@ -115,42 +186,47 @@
}
this.responseInterceptors.add(itcp);
}
-
+
/**
* Same as {@link #addResponseInterceptor addResponseInterceptor}.
*
* @param interceptor the interceptor to add
*/
public final
- void addInterceptor(final HttpResponseInterceptor interceptor) {
+ void addInterceptor(final HttpResponseInterceptor interceptor) {
addResponseInterceptor(interceptor);
}
-
-
+
+ public final void addInterceptor(final HttpResponseInterceptor interceptor, int index) {
+ addResponseInterceptor(interceptor, index);
+ }
+
+
+
// non-Javadoc, see interface HttpResponseInterceptorList
public int getResponseInterceptorCount() {
return (this.responseInterceptors == null) ?
0 : this.responseInterceptors.size();
}
-
-
+
+
// non-Javadoc, see interface HttpResponseInterceptorList
public HttpResponseInterceptor getResponseInterceptor(int index) {
if ((this.responseInterceptors == null) ||
- (index < 0) || (index >= this.responseInterceptors.size()))
+ (index < 0) || (index >= this.responseInterceptors.size()))
return null;
-
+
return (HttpResponseInterceptor) this.responseInterceptors.get(index);
}
-
-
+
+
// non-Javadoc, see interface HttpResponseInterceptorList
public void clearResponseInterceptors() {
this.responseInterceptors = null;
}
-
-
+
+
/**
* Sets the interceptor lists.
* First, both interceptor lists maintained by this processor
@@ -188,7 +264,7 @@
}
}
}
-
+
/**
* Clears both interceptor lists maintained by this processor.
*/
@@ -196,12 +272,12 @@
clearRequestInterceptors();
clearResponseInterceptors();
}
-
+
// non-Javadoc, see interface HttpRequestInterceptor (via HttpProcessor)
public void process(
final HttpRequest request,
- final HttpContext context)
- throws IOException, HttpException {
+ final HttpContext context)
+ throws IOException, HttpException {
if (this.requestInterceptors != null) {
for (int i = 0; i < this.requestInterceptors.size(); i++) {
HttpRequestInterceptor interceptor = (HttpRequestInterceptor) this.requestInterceptors.get(i);
@@ -209,12 +285,12 @@
}
}
}
-
+
// non-Javadoc, see interface HttpResponseInterceptor (via HttpProcessor)
public void process(
final HttpResponse response,
- final HttpContext context)
- throws IOException, HttpException {
+ final HttpContext context)
+ throws IOException, HttpException {
if (this.responseInterceptors != null) {
for (int i = 0; i < this.responseInterceptors.size(); i++) {
HttpResponseInterceptor interceptor = (HttpResponseInterceptor) this.responseInterceptors.get(i);
@@ -225,7 +301,7 @@
/**
* Creates a copy of this instance
- *
+ *
* @return new instance of the BasicHttpProcessor
*/
public BasicHttpProcessor copy() {
@@ -238,5 +314,5 @@
}
return clone;
}
-
+
}
Modified: jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpRequestInterceptorList.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpRequestInterceptorList.java?view=diff&rev=554367&r1=554366&r2=554367
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpRequestInterceptorList.java (original)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpRequestInterceptorList.java Sun Jul 8 06:40:19 2007
@@ -31,13 +31,10 @@
package org.apache.http.protocol;
-
import java.util.List;
import org.apache.http.HttpRequestInterceptor;
-
-
/**
* Provides access to an ordered list of request interceptors.
* Lists are expected to be built upfront and used read-only afterwards
@@ -54,14 +51,23 @@
/**
* Appends a request interceptor to this list.
*
- * @param itcp the request interceptor to add, or
- * <code>null</code> to do nothing
+ * @param itcp the request interceptor to add
*/
void addRequestInterceptor(HttpRequestInterceptor itcp)
;
/**
+ * Inserts a request interceptor at the specified index.
+ *
+ * @param itcp the request interceptor to add
+ * @param index the index to insert the interceptor at
+ * or MAX_INTEGER to insert last.
+ */
+ void addRequestInterceptor(HttpRequestInterceptor itcp, int index);
+
+
+ /**
* Obtains the current size of this list.
*
* @return the number of request interceptors in this list
@@ -90,6 +96,14 @@
;
+ /**
+ * Removes all request interceptor of the specified class
+ *
+ * @param clazz the class of the instances to be removed.
+ */
+ void removeRequestInterceptorByClass(Class clazz);
+
+
/**
* Sets the request interceptors in this list.
* This list will be cleared and re-initialized to contain
Modified: jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpResponseInterceptorList.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpResponseInterceptorList.java?view=diff&rev=554367&r1=554366&r2=554367
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpResponseInterceptorList.java (original)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/protocol/HttpResponseInterceptorList.java Sun Jul 8 06:40:19 2007
@@ -53,13 +53,21 @@
/**
* Appends a response interceptor to this list.
*
- * @param itcp the response interceptor to add, or
- * <code>null</code> to do nothing
+ * @param itcp the response interceptor to add
*/
void addResponseInterceptor(HttpResponseInterceptor itcp)
;
-
+ /**
+ * Inserts a response interceptor at the specified index.
+ *
+ * @param itcp the response interceptor to add
+ * @param index the index to insert the interceptor at
+ * or MAX_INTEGER to insert last.
+ */
+ void addResponseInterceptor(HttpResponseInterceptor itcp, int index);
+
+
/**
* Obtains the current size of this list.
*
@@ -88,7 +96,15 @@
void clearResponseInterceptors()
;
+
+ /**
+ * Removes all response interceptor of the specified class
+ *
+ * @param clazz the class of the instances to be removed.
+ */
+ void removeResponseInterceptorByClass(Class clazz);
+
/**
* Sets the response interceptors in this list.
* This list will be cleared and re-initialized to contain
Modified: jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestAllProtocol.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestAllProtocol.java?view=diff&rev=554367&r1=554366&r2=554367
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestAllProtocol.java (original)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestAllProtocol.java Sun Jul 8 06:40:19 2007
@@ -44,6 +44,7 @@
suite.addTest(TestStandardInterceptors.suite());
suite.addTest(TestHttpRequestHandlerRegistry.suite());
suite.addTest(TestHttpServiceAndExecutor.suite());
+ suite.addTest(TestBasicHttpProcessor.suite());
return suite;
}
Added: jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java?view=auto&rev=554367
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java (added)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java Sun Jul 8 06:40:19 2007
@@ -0,0 +1,165 @@
+/*
+ * BasicHttpProcessorTest.java
+ * JUnit based test
+ *
+ * Created on 1 July 2007, 11:07
+ */
+
+package org.apache.http.protocol;
+
+import java.io.IOException;
+
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpRequestInterceptor;
+
+/**
+ * @author Andrea Selva
+ */
+public class TestBasicHttpProcessor extends TestCase {
+
+ class TestHttpRequestInterceptorPlaceHolder implements HttpRequestInterceptor {
+
+ public void process(
+ HttpRequest request,
+ HttpContext context) throws HttpException, IOException {
+ }
+ }
+
+ // ------------------------------------------------------------ Constructor
+ public TestBasicHttpProcessor(String testName) {
+ super(testName);
+ }
+
+ // ------------------------------------------------------------------- Main
+ public static void main(String args[]) {
+ String[] testCaseName = { TestHttpExecutionContext.class.getName() };
+ junit.textui.TestRunner.main(testCaseName);
+ }
+
+ // ------------------------------------------------------- TestCase Methods
+
+ public static Test suite() {
+ return new TestSuite(TestHttpExecutionContext.class);
+ }
+
+ public void testAddFirstRequestInterceptorNull() {
+ HttpRequestInterceptor itcp = null;
+ BasicHttpProcessor instance = new BasicHttpProcessor();
+
+ instance.addRequestInterceptor(itcp, 0);
+ int itcpCount = instance.getRequestInterceptorCount();
+ assertEquals(0, itcpCount);
+ assertEquals(null, instance.getRequestInterceptor(0));
+ }
+
+ public void testAddFirsRequestInterceptor() {
+ HttpRequestInterceptor itcp1 = new HttpRequestInterceptor() {
+
+ public void process(
+ HttpRequest request,
+ HttpContext context) throws HttpException, IOException {
+ }
+
+ };
+ HttpRequestInterceptor itcp2 = new HttpRequestInterceptor() {
+
+ public void process(
+ HttpRequest request,
+ HttpContext context) throws HttpException, IOException {
+ }
+
+ };
+ BasicHttpProcessor instance = new BasicHttpProcessor();
+
+ instance.addRequestInterceptor(itcp1);
+ instance.addRequestInterceptor(itcp2, 0);
+ int itcpCount = instance.getRequestInterceptorCount();
+ assertEquals(2, itcpCount);
+ assertEquals(itcp2, instance.getRequestInterceptor(0));
+ }
+
+ public void testAddTailRequestInterceptorNull() {
+ System.out.println("testAddTailRequestInterceptorNull");
+
+ HttpRequestInterceptor itcp = null;
+ BasicHttpProcessor instance = new BasicHttpProcessor();
+
+ instance.addRequestInterceptor(itcp, 0);
+ int itcpCount = instance.getRequestInterceptorCount();
+ assertEquals(0, itcpCount);
+ assertEquals(null, instance.getRequestInterceptor(itcpCount - 1));
+ }
+
+ public void testAddTailRequestInterceptor() {
+ HttpRequestInterceptor itcp1 = new HttpRequestInterceptor() {
+
+ public void process(
+ HttpRequest request,
+ HttpContext context) throws HttpException, IOException {
+ }
+
+ };
+ HttpRequestInterceptor itcp2 = new HttpRequestInterceptor() {
+
+ public void process(
+ HttpRequest request,
+ HttpContext context) throws HttpException, IOException {
+ }
+
+ };
+ BasicHttpProcessor instance = new BasicHttpProcessor();
+
+ instance.addRequestInterceptor(itcp1);
+ instance.addRequestInterceptor(itcp2, Integer.MAX_VALUE);
+ int itcpCount = instance.getRequestInterceptorCount();
+ assertEquals(2, itcpCount);
+ assertEquals(itcp1, instance.getRequestInterceptor(0));
+ assertEquals(itcp2, instance.getRequestInterceptor(itcpCount - 1));
+
+ instance = new BasicHttpProcessor();
+ instance.addRequestInterceptor(itcp1, -1);
+ assertEquals(1, itcpCount);
+ assertEquals(itcp1, instance.getRequestInterceptor(0));
+ assertEquals(itcp1, instance.getRequestInterceptor(itcpCount - 1));
+ }
+
+ public void testAddRequestInterceptorMiddleIndex() {
+ HttpRequestInterceptor itcp1 = new TestHttpRequestInterceptorPlaceHolder();
+ BasicHttpProcessor instance = new BasicHttpProcessor();
+ instance.addRequestInterceptor(itcp1, 2);
+ }
+
+ public void testClearByClass() {
+ // remove a present class
+ HttpRequestInterceptor itcp1 = new TestHttpRequestInterceptorPlaceHolder();
+ HttpRequestInterceptor itcp2 = new TestHttpRequestInterceptorPlaceHolder();
+ HttpRequestInterceptor itcp3 = new HttpRequestInterceptor() {
+
+ public void process(
+ HttpRequest request,
+ HttpContext context) throws HttpException, IOException {
+ }
+
+ };
+ BasicHttpProcessor instance = new BasicHttpProcessor();
+ instance.addRequestInterceptor(itcp1);
+ instance.addRequestInterceptor(itcp2);
+ instance.addRequestInterceptor(itcp3);
+ instance.removeRequestInterceptorByClass(itcp1.getClass());
+ assertEquals(1, instance.getRequestInterceptorCount());
+ instance.removeRequestInterceptorByClass(itcp3.getClass());
+ assertEquals(0, instance.getRequestInterceptorCount());
+
+ // remove a not present class
+ instance.addRequestInterceptor(itcp1);
+ instance.addRequestInterceptor(itcp2);
+ instance.removeRequestInterceptorByClass(Integer.class);
+ assertEquals(2, instance.getRequestInterceptorCount());
+ }
+
+}
Propchange: jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java
------------------------------------------------------------------------------
svn:keywords = Date Author Id Revision HeadURL
Propchange: jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/protocol/TestBasicHttpProcessor.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk:
./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by Roland Weber <os...@dubioso.net>.
Hi Sebastian, Oleg,
> I think it is a bad idea to have to write calling code using 0 and
> Integer.MAX_VALUE as actual values. There should be named constants.
Actually, we already have an index-less method that will append.
So there is no need for Integer.MAX_VALUE. I've disabled the
magic number behavior. If you specify an index, it needs to be
in the valid range. Index 0 is always valid and represents the
start of the list.
bhp.addRequestInterceptor(icpt1); // add at end
bhp.addRequestInterceptor(icpt2, 0); // add at start
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk: ./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by sebb <se...@gmail.com>.
On 09/07/07, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Mon, 2007-07-09 at 13:56 +0200, Roland Weber wrote:
> > Hi Sebastian,
> >
> > > But in theory, one still has to use something like Integer.MAX_VALUE
> > > to absolutely guarantee that the entry will be added at the end...
> >
> > True. Now I remember why I wanted -1 to map to the end of the list.
> >
> > > If you don't want to use a magic number, why not have an addAtEnd() method?
> >
> > Because we already have too many methods in the interface to
> > manage two simple lists. *sigh*
> >
> > Add constants AT_END to the interfaces?
> > Or the extra methods? Or leave it as it is?
> >
>
> Roland,
>
> _Personally_ I do not simply see a legitimate use case for wanting to
> add a protocol interceptor at position three, five or three hundred and
> one. I just do not. One may legitimately want to have an interceptor
> executed first _before_ all other or last _after_ all others. That is
> it. To me #addXXXInterceptor(HttpXXXInterceptor itcp, int index) is
> pointless for all but two cases (0 or Integer.MAX_VALUE).
>
> Leave it as it is. It is good enough.
>
I'm inclined to agree with Oleg's use cases.
Though if you did want to get fancy with ordering, then I'd suggest
doing something like XStream which uses priorities.
For the simpler before/after case, perhaps use a two-valued indicator,
such as a boolean?
I think it is a bad idea to have to write calling code using 0 and
Integer.MAX_VALUE as actual values. There should be named constants.
S.
> Cheers,
>
> Oleg
>
>
>
> > cheers,
> > Roland
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in
/jakarta/httpcomponents/httpcore/trunk: ./
module-main/src/main/java/org/apache/http/protocol/
module-main/src/test/java/org/apache/http/protocol/
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2007-07-09 at 13:56 +0200, Roland Weber wrote:
> Hi Sebastian,
>
> > But in theory, one still has to use something like Integer.MAX_VALUE
> > to absolutely guarantee that the entry will be added at the end...
>
> True. Now I remember why I wanted -1 to map to the end of the list.
>
> > If you don't want to use a magic number, why not have an addAtEnd() method?
>
> Because we already have too many methods in the interface to
> manage two simple lists. *sigh*
>
> Add constants AT_END to the interfaces?
> Or the extra methods? Or leave it as it is?
>
Roland,
_Personally_ I do not simply see a legitimate use case for wanting to
add a protocol interceptor at position three, five or three hundred and
one. I just do not. One may legitimately want to have an interceptor
executed first _before_ all other or last _after_ all others. That is
it. To me #addXXXInterceptor(HttpXXXInterceptor itcp, int index) is
pointless for all but two cases (0 or Integer.MAX_VALUE).
Leave it as it is. It is good enough.
Cheers,
Oleg
> cheers,
> Roland
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk:
./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by Roland Weber <os...@dubioso.net>.
Hi Sebastian,
> But in theory, one still has to use something like Integer.MAX_VALUE
> to absolutely guarantee that the entry will be added at the end...
True. Now I remember why I wanted -1 to map to the end of the list.
> If you don't want to use a magic number, why not have an addAtEnd() method?
Because we already have too many methods in the interface to
manage two simple lists. *sigh*
Add constants AT_END to the interfaces?
Or the extra methods? Or leave it as it is?
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk: ./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by sebb <se...@gmail.com>.
On 09/07/07, Roland Weber <os...@dubioso.net> wrote:
> Oleg Kalnichevski wrote:
> > I would actually expect a IndexOutOfRangeExceptions in case of an
> > invalid index. What if one passes -1 by mistake and gets the interceptor
> > quietly appended to the list last?
>
> I see. Good point.
>
> > For the same reason I do not quite
> > like the idea of using Integer.MAX_VALUE, but I have promised to myself
> > to not get into API purity discussions with you.
>
> I don't like Integer.MAX_VALUE as a magic number either.
> I wrote it only as an example in my JIRA comment, since
> I would map everything beyond the end of the list to
> the end of the list. If we define a single magic number,
> it should be defined as a constant in our interface.
>
> How about mapping all positive values beyond the end to
> the end of the list, but throwing an exception for
> negatives? That avoids unexpected wrap-around behavior
> near 0, but also avoids querying the size of the list
> if all you want to do is adding something at the end:
But in theory, one still has to use something like Integer.MAX_VALUE
to absolutely guarantee that the entry will be added at the end...
If you don't want to use a magic number, why not have an addAtEnd() method?
Just my 2p.
> if (index < 0)
> throw IndexOutOfBoundsException(...);
> if (index > ...size())
> index = ...size();
>
> cheers,
> Roland
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk:
./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by Roland Weber <os...@dubioso.net>.
Oleg Kalnichevski wrote:
> On Mon, 2007-07-09 at 11:02 +0200, Roland Weber wrote:
>>
>> if (index < 0)
>> throw IndexOutOfBoundsException(...);
>> if (index > ...size())
>> index = ...size();
>
> Cool. Sounds reasonable to me.
I'll take care of it.
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 -
in /jakarta/httpcomponents/httpcore/trunk:
./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2007-07-09 at 11:02 +0200, Roland Weber wrote:
> Oleg Kalnichevski wrote:
> > I would actually expect a IndexOutOfRangeExceptions in case of an
> > invalid index. What if one passes -1 by mistake and gets the interceptor
> > quietly appended to the list last?
>
> I see. Good point.
>
> > For the same reason I do not quite
> > like the idea of using Integer.MAX_VALUE, but I have promised to myself
> > to not get into API purity discussions with you.
>
> I don't like Integer.MAX_VALUE as a magic number either.
> I wrote it only as an example in my JIRA comment, since
> I would map everything beyond the end of the list to
> the end of the list. If we define a single magic number,
> it should be defined as a constant in our interface.
>
> How about mapping all positive values beyond the end to
> the end of the list, but throwing an exception for
> negatives? That avoids unexpected wrap-around behavior
> near 0, but also avoids querying the size of the list
> if all you want to do is adding something at the end:
>
> if (index < 0)
> throw IndexOutOfBoundsException(...);
> if (index > ...size())
> index = ...size();
>
> cheers,
> Roland
>
Cool. Sounds reasonable to me.
Cheers
Oleg
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk:
./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by Roland Weber <os...@dubioso.net>.
Oleg Kalnichevski wrote:
> I would actually expect a IndexOutOfRangeExceptions in case of an
> invalid index. What if one passes -1 by mistake and gets the interceptor
> quietly appended to the list last?
I see. Good point.
> For the same reason I do not quite
> like the idea of using Integer.MAX_VALUE, but I have promised to myself
> to not get into API purity discussions with you.
I don't like Integer.MAX_VALUE as a magic number either.
I wrote it only as an example in my JIRA comment, since
I would map everything beyond the end of the list to
the end of the list. If we define a single magic number,
it should be defined as a constant in our interface.
How about mapping all positive values beyond the end to
the end of the list, but throwing an exception for
negatives? That avoids unexpected wrap-around behavior
near 0, but also avoids querying the size of the list
if all you want to do is adding something at the end:
if (index < 0)
throw IndexOutOfBoundsException(...);
if (index > ...size())
index = ...size();
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in
/jakarta/httpcomponents/httpcore/trunk: ./
module-main/src/main/java/org/apache/http/protocol/
module-main/src/test/java/org/apache/http/protocol/
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2007-07-08 at 15:51 +0200, Roland Weber wrote:
> Sorry, I hadn't reviewed that patch before...
>
> > + // non-Javadoc, see interface HttpRequestInterceptorList
> > + public void addRequestInterceptor(final HttpRequestInterceptor itcp, int index) {
> > +
> > + if (itcp == null) {
> > + return;
> > + }
> > + if (this.requestInterceptors == null) {
> > + this.requestInterceptors = new ArrayList();
> > + }
> > +
> > + if (index == Integer.MAX_VALUE) {
> > + // Add last
> > + this.requestInterceptors.add(this.requestInterceptors.size() - 1, itcp);
> > + } else {
> > + // Insert at preferred index
> > + this.requestInterceptors.add(index, itcp);
> > + }
> > + }
>
> This does leave room for IndexOutOfRangeExceptions, which is not
> what I had in mind. Can you please change that to...
>
> if ((index < 0) || (index > this.requestInterceptors.size())
> index = this.requestInterceptors.size();
> this.requestInterceptors.add(index, itcp);
>
> and likewise for the response interceptors?
>
Roland,
I would actually expect a IndexOutOfRangeExceptions in case of an
invalid index. What if one passes -1 by mistake and gets the interceptor
quietly appended to the list last? For the same reason I do not quite
like the idea of using Integer.MAX_VALUE, but I have promised to myself
to not get into API purity discussions with you.
I'll make the change if you think it is what you want.
Oleg
> cheers,
> Roland
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: svn commit: r554367 - in /jakarta/httpcomponents/httpcore/trunk:
./ module-main/src/main/java/org/apache/http/protocol/ module-main/src/test/java/org/apache/http/protocol/
Posted by Roland Weber <os...@dubioso.net>.
Sorry, I hadn't reviewed that patch before...
> + // non-Javadoc, see interface HttpRequestInterceptorList
> + public void addRequestInterceptor(final HttpRequestInterceptor itcp, int index) {
> +
> + if (itcp == null) {
> + return;
> + }
> + if (this.requestInterceptors == null) {
> + this.requestInterceptors = new ArrayList();
> + }
> +
> + if (index == Integer.MAX_VALUE) {
> + // Add last
> + this.requestInterceptors.add(this.requestInterceptors.size() - 1, itcp);
> + } else {
> + // Insert at preferred index
> + this.requestInterceptors.add(index, itcp);
> + }
> + }
This does leave room for IndexOutOfRangeExceptions, which is not
what I had in mind. Can you please change that to...
if ((index < 0) || (index > this.requestInterceptors.size())
index = this.requestInterceptors.size();
this.requestInterceptors.add(index, itcp);
and likewise for the response interceptors?
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org