You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2012/09/02 12:52:34 UTC

Re: svn commit: r1379856 - in /jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol: mail/sampler/MailFileFolder.java smtp/sampler/protocol/TrustAllSSLSocketFactory.java

On 1 September 2012 22:10,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Sat Sep  1 21:10:19 2012
> New Revision: 1379856
>
> URL: http://svn.apache.org/viewvc?rev=1379856&view=rev
> Log:
> method returning arrays should return empty array instead of null

Ideally yes, but it depends on how the API was defined.
There may be cases where null is treated differently from an empty array.

In this case it appears that empty arrays are expected and null is
not, so the change is OK.

> Modified:
>     jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
>     jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
>
> Modified: jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java?rev=1379856&r1=1379855&r2=1379856&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java (original)
> +++ jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java Sat Sep  1 21:10:19 2012
> @@ -90,7 +90,7 @@ public class MailFileFolder extends Fold
>
>      @Override
>      public Message[] expunge() throws MessagingException {
> -        return null;
> +        return new Message[0];
>      }
>
>      @Override
>
> Modified: jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java?rev=1379856&r1=1379855&r2=1379856&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java (original)
> +++ jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java Sat Sep  1 21:10:19 2012
> @@ -46,7 +46,7 @@ public class TrustAllSSLSocketFactory ex
>              sslcontext.init( null, new TrustManager[]{
>                      new X509TrustManager() {
>                          public java.security.cert.X509Certificate[] getAcceptedIssuers() {
> -                            return null;
> +                            return new java.security.cert.X509Certificate[0];
>                          }
>                          public void checkClientTrusted(
>                                  java.security.cert.X509Certificate[] certs, String authType) {
>
>

Re: svn commit: r1379856 - in /jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol: mail/sampler/MailFileFolder.java smtp/sampler/protocol/TrustAllSSLSocketFactory.java

Posted by sebb <se...@gmail.com>.
On 2 September 2012 12:48, Philippe Mouawad <ph...@gmail.com> wrote:
> Hello sebb,
> I checked api before doing changes and you are right , for example ssl
> related api expects null so i didn't change.

Which API is that? It would be useful to document the instances that
can return null.

> But if you see something wrong, revert the change.

The changes in this commit are OK, but in future I think we need to
document why such fixes are OK, either in log message, or better in
the code.

> Thanks for review
> Regards
> Philippe
>
> On Sunday, September 2, 2012, sebb wrote:
>
>> On 1 September 2012 22:10,  <pmouawad@apache.org <javascript:;>> wrote:
>> > Author: pmouawad
>> > Date: Sat Sep  1 21:10:19 2012
>> > New Revision: 1379856
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1379856&view=rev
>> > Log:
>> > method returning arrays should return empty array instead of null
>>
>> Ideally yes, but it depends on how the API was defined.
>> There may be cases where null is treated differently from an empty array.
>>
>> In this case it appears that empty arrays are expected and null is
>> not, so the change is OK.
>>
>> > Modified:
>> >
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
>> >
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
>> >
>> > Modified:
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java?rev=1379856&r1=1379855&r2=1379856&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
>> (original)
>> > +++
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
>> Sat Sep  1 21:10:19 2012
>> > @@ -90,7 +90,7 @@ public class MailFileFolder extends Fold
>> >
>> >      @Override
>> >      public Message[] expunge() throws MessagingException {
>> > -        return null;
>> > +        return new Message[0];
>> >      }
>> >
>> >      @Override
>> >
>> > Modified:
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java?rev=1379856&r1=1379855&r2=1379856&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
>> (original)
>> > +++
>> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
>> Sat Sep  1 21:10:19 2012
>> > @@ -46,7 +46,7 @@ public class TrustAllSSLSocketFactory ex
>> >              sslcontext.init( null, new TrustManager[]{
>> >                      new X509TrustManager() {
>> >                          public java.security.cert.X509Certificate[]
>> getAcceptedIssuers() {
>> > -                            return null;
>> > +                            return new
>> java.security.cert.X509Certificate[0];
>> >                          }
>> >                          public void checkClientTrusted(
>> >                                  java.security.cert.X509Certificate[]
>> certs, String authType) {
>> >
>> >
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: svn commit: r1379856 - in /jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol: mail/sampler/MailFileFolder.java smtp/sampler/protocol/TrustAllSSLSocketFactory.java

Posted by Philippe Mouawad <ph...@gmail.com>.
Hello sebb,
I checked api before doing changes and you are right , for example ssl
related api expects null so i didn't change.

But if you see something wrong, revert the change.

Thanks for review
Regards
Philippe

On Sunday, September 2, 2012, sebb wrote:

> On 1 September 2012 22:10,  <pmouawad@apache.org <javascript:;>> wrote:
> > Author: pmouawad
> > Date: Sat Sep  1 21:10:19 2012
> > New Revision: 1379856
> >
> > URL: http://svn.apache.org/viewvc?rev=1379856&view=rev
> > Log:
> > method returning arrays should return empty array instead of null
>
> Ideally yes, but it depends on how the API was defined.
> There may be cases where null is treated differently from an empty array.
>
> In this case it appears that empty arrays are expected and null is
> not, so the change is OK.
>
> > Modified:
> >
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
> >
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
> >
> > Modified:
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java?rev=1379856&r1=1379855&r2=1379856&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
> (original)
> > +++
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/mail/sampler/MailFileFolder.java
> Sat Sep  1 21:10:19 2012
> > @@ -90,7 +90,7 @@ public class MailFileFolder extends Fold
> >
> >      @Override
> >      public Message[] expunge() throws MessagingException {
> > -        return null;
> > +        return new Message[0];
> >      }
> >
> >      @Override
> >
> > Modified:
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java?rev=1379856&r1=1379855&r2=1379856&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
> (original)
> > +++
> jmeter/trunk/src/protocol/mail/org/apache/jmeter/protocol/smtp/sampler/protocol/TrustAllSSLSocketFactory.java
> Sat Sep  1 21:10:19 2012
> > @@ -46,7 +46,7 @@ public class TrustAllSSLSocketFactory ex
> >              sslcontext.init( null, new TrustManager[]{
> >                      new X509TrustManager() {
> >                          public java.security.cert.X509Certificate[]
> getAcceptedIssuers() {
> > -                            return null;
> > +                            return new
> java.security.cert.X509Certificate[0];
> >                          }
> >                          public void checkClientTrusted(
> >                                  java.security.cert.X509Certificate[]
> certs, String authType) {
> >
> >
>


-- 
Cordialement.
Philippe Mouawad.