<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Max,<br>
<br>
Thanks for your comments ,please check updated webrev and my
replies inline. <br>
webrev: <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eamjiang/8050427/webrev.02/">http://cr.openjdk.java.net/~amjiang/8050427/webrev.02/</a>
<br>
<br>
Regards,<br>
Amanda<br>
<br>
On 8/17/15, 12:29 AM, Weijun Wang wrote:<br>
</div>
<blockquote cite="mid:55D18D5A.7020207@oracle.com" type="cite">There
are something I don't understand. <br>
<br>
MyConfiguration.java: <br>
<br>
- optionOrder: Is it possible to make this an argument of the
constructor? <br>
</blockquote>
Fixed, made this an argument of the constructor.<br>
<blockquote cite="mid:55D18D5A.7020207@oracle.com" type="cite"> <br>
- getConfiguration/setConfiguration: If these are useless, why
adding these methods? <br>
</blockquote>
Removed useless methods.<br>
<blockquote cite="mid:55D18D5A.7020207@oracle.com" type="cite"> <br>
SmartLoginModule.java: <br>
<br>
- initialize: Why don't you use the callbackHandler argument? <br>
</blockquote>
See below.<br>
<blockquote cite="mid:55D18D5A.7020207@oracle.com" type="cite"> <br>
- shouldSucceed: It is always very confusing to make a field
accessible from outside a class. <br>
</blockquote>
Originally the test tried to test with different password ,
"shouldSucceed" is used to control which password will be set.<br>
MycallbackHandler is called in initialize( ) method and
"shouldSucceed" is parsed as an argument to control which password
would be set to "PasswdCallback"<br>
<meta charset="utf-8">
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> 123 this.callbackHandler = new MyCallbackHandler(myUsername, myPassword,
124 shouldSucceed);
<meta charset="utf-8">341 public MyCallbackHandler(String username, char[] password, boolean action) {
342 super();
343 userName = username;
344 this.password = password;
345 this.action = action;
346 }</pre>
<meta charset="utf-8">
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> 361 PasswordCallback pc = (PasswordCallback) callback;
362 if (action) {
363 pc.setPassword(password);
364 } else {
365 pc.setPassword(wrongpd);
366 }
</pre>
I agree with you this may be confusing, so I simplify this test,
"shouldSucceed" value is remove, please check updated webrev.<br>
<blockquote cite="mid:55D18D5A.7020207@oracle.com" type="cite"> <br>
DynamicConfigurationTest.java: <br>
<br>
- test: why the if checks are based on both isNegative and
success? Why not only on isNegative? If you want to test 2 stages
(initialize and login), you can provide 2 isNegative flags. <br>
</blockquote>
"success" was for controlling correct/wrong passwords,
"isNegative" was for checking if exception is expected.<br>
Tests are re-organized , so I do not use these two flags
now, please check updated webrev. <br>
<blockquote cite="mid:55D18D5A.7020207@oracle.com" type="cite"> <br>
Thanks <br>
Max <br>
<br>
<br>
On 08/17/2015 01:39 PM, Amanda Jiang wrote: <br>
<blockquote type="cite">Hi All, <br>
<br>
Please be free to review these new tests for Dynamic
configuration of <br>
authentication modules. <br>
<br>
Bug: <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8050427">https://bugs.openjdk.java.net/browse/JDK-8050427</a>
<br>
webrev:<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eamjiang/8050427/webrev.01/">http://cr.openjdk.java.net/~amjiang/8050427/webrev.01/</a>
<br>
<br>
<br>
Thanks, <br>
Amanda <br>
<br>
<br>
</blockquote>
</blockquote>
<br>
<br>
</body>
</html>