[patch] integration test

[GoogleCodeExporter]

Type-Patch

* include in SampleConsumer and SampleServer, sample usage of Attribute and
SReg
* add 2 integration tests (using SampleConsumer, SampleServer)
* add 1 integration test for integration test with external provider.  The
sample test try an authentification against videntity.org, authentification
sucessed but retreive information (email) failed (commented).
* improve test coverage from 23% to 51%

!! I haven't patch lib directory and build.xml (integration test require
jwebunit-1.4.1)

Original issue reported on code.google.com by david.bernard.31 on 17 Oct 2007 at 10:30

Attachments:

• maven2.patch
• src.patch
• test.patch

[GoogleCodeExporter]

Original comment by Johnny.B...@gmail.com on 17 Oct 2007 at 5:33

• Changed state: Started

[GoogleCodeExporter]

Patch to AuthRequest applied, thanks!

Original comment by Johnny.B...@gmail.com on 17 Oct 2007 at 5:37

[GoogleCodeExporter]

--- src/org/openid4java/message/ParameterList.java      (revision 360)
+++ src/org/openid4java/message/ParameterList.java      (working copy)
-            String keyValue = tokenizer.nextToken();
+            String keyValue = tokenizer.nextToken().trim();

What's the basis for trimming the key/values that (potentialy a different) 
party has 
put into an OpenID message?

+            if (keyValue.length() == 0) {
+                continue;
+            }

This seems to be not needed; "".indexOf("<anything>") is -1, which is the very 
next 
text.

Original comment by Johnny.B...@gmail.com on 17 Oct 2007 at 5:38

[GoogleCodeExporter]

Added patch to Sample Consumer / Server. 

Removed lastEmailFromFetch and lastEmailFromSReg members from SampleConsumer: 
security risk, as the SampleConsumer is used to process all transactions, thus 
the 
values stored in the members could potentially be returned to the wrong 
user(s). 
Replaced with TODO note, to be handled appropriately by whoever ends up using 
the 
sample consumer code.

Original comment by Johnny.B...@gmail.com on 17 Oct 2007 at 7:09

[GoogleCodeExporter]

1. the original test in ParameterList was
   if (keyValue.trim().length() == 0)
   {
       continue;
   }
   As shown by the test added to ParameterListTest, I add it because when I play with
simple-openid I got error due to reponse with blank line (space + newline). So 
tho
avoid parsing blank line, I added the control

2. Your right about lastEmailFromSReg and lastEmailFromFecth, it's why I call 
them
lastXxxx and not Xxxx. SampleConsumer is a demo purpose (not a production, 
ready to
use class(multi-user with security,...)). And, to be a valid demo, I used it for
test. And lastXxxx are enough for demo, and single threaded test. using 
lastXxx, also
allow to avoid create a UserData structure/map/... If you want a more 
"realistic"
sample, We could replace lastXxx by entries in the Session ?

Original comment by david.bernard.31 on 17 Oct 2007 at 9:23

[GoogleCodeExporter]

I've replace lastXxxx by Session's attribute, and I also reconfigure formatting 
for
new line on open brace. Do you want I resubmit the patch ?

Original comment by david.bernard.31 on 17 Oct 2007 at 9:55

[GoogleCodeExporter]

1. 
If the response is invalid and generates an error, then the party that is 
sending it 
should be fixed so that it sends a valid response. Modifying the response on 
the 
receiving end can lead to security issues. 
If a workaround is needed for a specific case (to improve user experience with 
a 
widely deployed bug) it shouldn't be dealt with in the library, where it can 
affect 
users that may not want to deal with it in the same way.

2. 
Agreed that the sample code is mainly for demo purposes. However website 
developers 
(especially the ones not familiar with OpenID) may take it and deploy to 
production.

Storing the results in the session or modifying the return data type of the 
method 
should work better I think.

Original comment by Johnny.B...@gmail.com on 17 Oct 2007 at 9:58

[GoogleCodeExporter]

David, can you please try to commit them yourself? I've added you to the 
project 
members. Not sure if it will work with your account or if you need to sign up 
for a 
google account, but please give it a try and let me know. The SVN password can 
is 
generated at this URL: http://code.google.com/hosting/settings

Thanks!

Original comment by Johnny.B...@gmail.com on 18 Oct 2007 at 1:08

[GoogleCodeExporter]

Thanks Johnny,

I successfully committed my first patch into openid4java.

:-)

Original comment by david.bernard.31 on 18 Oct 2007 at 6:14

• Changed state: Fixed
• Added labels: Type-Patch
• Removed labels: Type-Defect

[GoogleCodeExporter]

David,

It looks like your commit for rev 367 broke the tests:
http://opensource.bamboo.atlassian.com/browse/OPENID-OPENID4JAVA

Can you please see what's wrong? I also get compilation error when I try to run 
the 
tests on my machine.

Thanks!

Original comment by Johnny.B...@gmail.com on 18 Oct 2007 at 5:35

• Changed state: Started

[GoogleCodeExporter]

It's a dependncy problem undefined into ant's build.xml.
I'll fixed it. (I need to install ant ;))

Original comment by david.bernard.31 on 18 Oct 2007 at 7:11

[GoogleCodeExporter]

What is your opinion about using ivy (http://ant.apache.org/ivy/) to manage
dependency for ant build. I could do a try/start for the test.

Using tool like ivy or maven for managing dependencies (and transitive) will 
avoid
storing several version (not fully sync) of the same lib for different part of 
the
project. It will also avoid managing (clean, update) transitive deps (not 
directly
used by the project).

What is your opinion ?

An other solution, switch to maven ;)

Original comment by david.bernard.31 on 18 Oct 2007 at 9:35

[GoogleCodeExporter]

Using IVY should be ok. I actually looked at it a few weeks ago, but decided 
not to 
bother since there aren't that many dependencies. The standing requirement is 
to not 
complicate things for someone new who downloads the package: 'ant jar' / 'ant 
test' 
etc need to work out of the box.

Maven is okay as well. If there's enough demand and commitment there's no 
reason not 
to use it. However, the maven:ant preference ratio must become overwhelming 
before 
deciding to drop ant and force everyone to switch to maven.

Original comment by johnny%s...@gtempaccount.com on 19 Oct 2007 at 3:34

[GoogleCodeExporter]

I didn't use ant since 2 or 3 years, but after some try using Ivy for test
dependencies resolution seems to work (I committed and bamboo success).
No need to install ivy, do change into ant installation (tested with 1.7).

Original comment by david.bernard.31 on 19 Oct 2007 at 10:33

• Changed state: Fixed

[GoogleCodeExporter]

I'm stupid. I spare lot of time to learn ivy, when a maven antlib could do it 
simply.
No we have 4 builder options :
* ant+jar
* ant+ivy
* ant+maven2-antlib
* maven2

;)

Original comment by david.bernard.31 on 19 Oct 2007 at 8:13