harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yang Paulex" <paulex.y...@gmail.com>
Subject Re: [classlib] please review my regression test and patch for HttpURLConnection
Date Thu, 25 May 2006 09:27:21 GMT
Mikhail,

Sorry to response so late, seems both IBM's smtp server and gmail is not
stable so that I have to send this mail several times from today before
yesterday (please pardon me if the mailing list get several duplicate mails
after hundreds of hours! ).

Basically, I'm afraid I cannot agree with you on this patch.

1. I have no strong objection on the patch for HttpURLConnection, but...If I
understand correctly, Harmony-482 is about the internal contract between
java.net.HttpURLConnection.setRequestMethod() and
o.a.h.l.internal.net.www.protocol.http.HttpURLConnection.getOutputStream(),
and the internal contract may be broken only if some of the codes refactored
(no chance for user application), say, modification for the
setRequestMethod() like this:

-               this.method = methodTokens[i];
+                this.method = method;

But this can be easily monitored by test codes below:

   public void testGetOutputStream() throws Exception {
       HttpURLConnection c = (HttpURLConnection) new
URL("http://127.0.0.1:"<http://127.0.0.1/>
               + port).openConnection();
       c.setDoOutput(true);
       c.setRequestMethod(new String("GET"));
       c.getOutputStream();

       ...//other for POST/PUT
   }

This test is enough to enforced the internal contract, it will shout if some
refactory breaks the internal contract. So IMHO, I think the patch for
HttpURLConnection is a little over-designed.

2. Even the patch is necessary, I think the introduction of injected
java.net.HttpURLConnectionAccessor is debatable. I suggest to avoid the
injected helper class as long as possible, because it introduces complexity
to understand/manage and is sometimes fragile because it may highly coupled
with implementation details. For this specific case, a mocked subclass of
o.a.h....HttpURLConnection  with overridden getRequestMethod can be used
instead.

Your comments?


2006/5/23, Mikhail Loenko <mloenko@gmail.com>:
>
> I've created regression test and patch for
> http://issues.apache.org/jira/browse/HARMONY-482
>
> I had to make some changes in the luni's build.xml,
> that are intended to be fixed once we all agree with
> proposed test suite layout.
>
> Please review the changes.
>
> Thanks,
> Mikhail
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>


-- 
Paulex Yang
China Software Development Labotary
IBM

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message