@play.data.validation.Password creates <input type="text" />

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Create a model class that extends SienaSupport
2. Add a String field with @play.data.validation.Password
3. Create CRUD controller for the model just created

What is the expected output? What do you see instead?
Instead of a <input type="password" /> a <input type="text" /> is created for 
the password field

What version of the product are you using? On what operating system?
Windows 7 x64
Play 1.1
GAE 1.4
Siena 1.3
Crudsiena 1.2

The fix is pretty simple:
In controllers.CRUD on line 491 add this code:
if(field.isAnnotationPresent(Password.class)) {
  type = "password";
}

Original issue reported on code.google.com by spreiter...@gmail.com on 10 Dec 2010 at 8:47

[GoogleCodeExporter]

Yes exactly,
you're right!
I'm going to correct this now!

Thanks for your help
Pascal

Original comment by pascal.v...@gmail.com on 11 Dec 2010 at 5:56

• Changed state: Accepted

[GoogleCodeExporter]

Glad I could help you.

I've created a Unique annotation that works with your moduel

UniqueCheck.java
http://pastie.org/1362809

Unique.java
http://pastie.org/1362797

Example
http://pastie.org/1367272

Maybe you want to include it.

Thanks for this great module!
Herny

Original comment by spreiter...@gmail.com on 11 Dec 2010 at 6:10

[GoogleCodeExporter]

Great contribution :)
I'm going to integrate this in the new version with the bug correction.
I will have a look also if I can find a way to remove the need to fill the 
"field" attribute in the annotation.
Thank you very much!

Your welcome for other contributions ;)

regards
Pascal

Original comment by pascal.v...@gmail.com on 11 Dec 2010 at 6:28

[GoogleCodeExporter]

Yeah would be great if you can figure it out.

If you need a helping hand let me know :)

regards
Henry

Original comment by spreiter...@gmail.com on 11 Dec 2010 at 8:20

[GoogleCodeExporter]

I have integrated your code and changed some little part to make it more 
generic (for example, the Check doesn't depend on SienaSupport now but directly 
on Siena.Model).
Moreover, I renames the annotation @CrudUnique as it is a CRUD validation and 
shouldn't be mistaken for the @Siena.Unique. The annotation doesn't need the 
attribute "field" also.
I haven't yet committed the code because I'm thinking about something: this 
annotation is useful in Crud but it hides some costly DB.filter.fetch. So if it 
applies automatically outside the CRUDSiena Module it could bring a hidden 
problem of performance.
What do you think about this issue?

br
Pascal

Original comment by pascal.v...@gmail.com on 13 Dec 2010 at 9:53

[GoogleCodeExporter]

It's a valid concern. Is it somehow possible to set a flag when the field is 
set so it only checks when the field change? I'll have a look at it.

If not I think it's up to the developer to decide when to apply this 
annotation. We should clarify that CrudUnique isn't the same as a DB Unique 
constraint. It's more a convenient way for checking if some value is already in 
the database instead of doing it by hand with the drawback that it does this 
check for every update.

I already thought about this problem when I've written this annotation but it 
wasn't a problem because it is an entity that is never updated again.

regards
Henry

Original comment by spreiter...@gmail.com on 13 Dec 2010 at 4:22

[GoogleCodeExporter]

Hi, I have just delivered crudsiena-1.3 with @Password and @CrudUnique (and its 
warning in the doc)
Test it and tell me if it works for you!

thanks
Pascal

Original comment by pascal.v...@gmail.com on 14 Dec 2010 at 10:27

• Changed state: Fixed

[GoogleCodeExporter]

I will test it this evening and let you know if it works :)

regards
Henry

Original comment by spreiter...@gmail.com on 15 Dec 2010 at 4:09

[GoogleCodeExporter]

I just had an idea for the performance problem:
What if we use plays caching inside CrudUniqueCheck?
Something like this:

Object sskey = SienaUtils.findKey(ss);
FieldContext ctx = (FieldContext) context;
String fieldName = ctx.getField().getName();
String className = ctx.getCompileTimeType().getName();
if(sskey != null) {
 // existing object. check if value is inside Cache and if it's equal to the current
 String key = className + "." + fieldName + "." + sskey.toString();
 String oldValue = Cache.get(key);
 if(value != null && value.equals(oldValue) {
  // same value. no need to check inside database
  return true;
 }
}

What do you think?

Oh btw.: I just saw you wrote that there will be Fixture support in a future 
release. I've added Fixutre support (SienaFixture) in the Siena play module and 
it got merged into the master 2 days ago ;)

regards Henry

Original comment by spreiter...@gmail.com on 15 Dec 2010 at 5:39

[GoogleCodeExporter]

hi,
your idea of the cache might reduce a bit the load... good idea (I had not 
thought about it at all... I still discover deep Play features :))...
Anyway, I really would like to be able to trigger this annotation when I need 
it or not... a kind of flag that can be set on/off in an action for example... 
I wonder which place would be the best for that...

Concerning the SienaFixtures, this is greatt!!! Is it realeased in a new 
version of the Siena module? Does it implement everything? the deleteall, the 
YAML etc...?

If yes, I'd like to add a new page with an URL such as "/bulk" to the CrudSiena 
with 2 buttons : deleteAll, provisionAll... this is so practical :)

thanks anyway!
Pascal

Original comment by pascal.v...@gmail.com on 15 Dec 2010 at 9:18

[GoogleCodeExporter]

Discovering new features ... the beauty of open source :) I am also very new to 
play and siena (not even 2 weeks).

Yeah a trigger for it would be great in addition to caching. I'll have a look 
at it.

Concerning the SienaFixture: It only implements deleteAll and load (it was all 
I needed at the time). Also there is a tag for the selenium test 
#{siena.sienafixture load:'my.yml', delete:'all' /}

though the deleteAll might need some rework because all it does is to read the 
files inside app/models, load the class, fetch all records and deletes them one 
by one.
https://github.com/vijaykiran/play-siena/blob/master/src/play/modules/siena/Sien
aFixture.java

Classes inside app/models/subpackges don't get deleted but I think I will 
implement it in a new way. As I learned the play classloader has all the 
classes stored so I can itereate over them instead of using files.

Also it is not yet released but the master is basically Siena 1.3 + 
SienaFixture.java + sienafixture.html

the /bulk idea sounds good and practical :)

regards
Henry

Original comment by spreiter...@gmail.com on 15 Dec 2010 at 9:52

[GoogleCodeExporter]

I think I have a solution for the trigger. If we use ThreadLocale we could 
create an Object with a flag that we can check for inside CrudUniqueCheck. 
Pseudo Code:

// inside CrudUniqueCheck.java
private static final ThreadLocal<ObjectWithFlag> current = new 
ThreadLocal<ObjectWithFlag>() {
 @Override protected ObjectWithFlag initialValue() {
 return new ObjectWithFlag(); // init the object with flag set to true so check will be executed for the current thread
}
public static void disableUniqueCheck() {
 current.get().disableUniqueCheck(); // set the flag to false so no check will be executed for the current thread
}
@Override
public boolean isSatisfied(Object validatedObject, Object value, OValContext 
context, Validator validator) throws OValException {
 if(current.get().executeCheck()) {
  // do the check
 } else {
  // if executeCheck returns false, the flag will be reseted to true
  return true;
 }
}

now inside a controller we could to something like this:
CrudUniqueCheck.disableUniqueCheck();
// now some code that saves the entity

that's it.

What do you think about that?

Herny

Original comment by spreiter...@gmail.com on 15 Dec 2010 at 4:00

[GoogleCodeExporter]

yes this might be a nice idea! A few questions:
Is initialValue() called automatically by the validation manager?

Did you look at the allocation model of CrudUniqueCheck? All action params are 
threadlocal in the controller afaik. But for validators and validation objects, 
I don't know. I wonder whether setting a threadlocal in a validator is advised 
or not.

I need to look at the code because I know everything is stateless but not 
exactly how :)

Original comment by pascal.v...@gmail.com on 15 Dec 2010 at 4:15

[GoogleCodeExporter]

If current.get() is called the first time, initialValue() is called 
automatically only once. After that, get() returns the same value over and over 
again for that Thread. see ThreadLocale for more information 
http://download.oracle.com/javase/6/docs/api/java/lang/ThreadLocal.html

request/response are managed by ThreadLocale (see 
https://github.com/playframework/play/blob/master/framework/src/play/mvc/Http.ja
va).  

>> Did you look at the allocation model of CrudUniqueCheck?
No but I think it doesn't matter since "current" is static so it exists only 
once (like request/response in Http)

Or do you know another way we could handle it? It seems to be the right way 
since play handles requests in the same fashion with ThreadLocale

Original comment by spreiter...@gmail.com on 15 Dec 2010 at 5:22

[GoogleCodeExporter]

Don't worry, I don't say you're wrong :)
I like your way. I'm just trying to see if something could be wrong with it!

I know threadlocal. 

My question was in fact: are we sure the validation thread is the same as the 
controller thread? but I really don't see why it wouldn't be like that!

With this and the cache, I think CrudUnique is quite powerful :)

thanks
Pascal

Original comment by pascal.v...@gmail.com on 15 Dec 2010 at 5:33

[GoogleCodeExporter]

I just tested it in my project

before validateAndSave() controller Thread ID 15
call isSatisfied Thread ID 15
before validateAndSave() controller Thread ID 15

It's kept in the same Thread for the request :)

What happens if we have an exception before we can reset the flag inside 
isSatisfied?

Example:

CrudUniqueCheck.disable();
// some code that raises an exception => flag not reset because isSatisfied is 
never called

We need to make sure that the flag is always reseted. Or should we leave it to 
the developer to explicit call something like CrudUniqueCheck.reset();?

have a nice evening
Henry

Original comment by spreiter...@gmail.com on 15 Dec 2010 at 6:20

[GoogleCodeExporter]

Good question! I think we need to reset the flag automatically!
For the next incoming request that calls the validation, do we keep the same 
instance of CrudUniqueCheck and thread? Isn't it a new instance with the flag 
reseted?
I will test that... tomorrow I'm quite busy, maybe in the evening...

thanks for your work
Pascal

Original comment by pascal.v...@gmail.com on 15 Dec 2010 at 11:41

[GoogleCodeExporter]

Hi,

sorry was busy yesterday.

I tested it: For every validateAndSave() call a new instance of CrudUniqueCheck 
is created.

So we need to consider this as well.

Original comment by spreiter...@gmail.com on 17 Dec 2010 at 6:01

[GoogleCodeExporter]

Me too, my yesterday was crazy :)

This is the beauty of the stateless model :)
Why not create a new issue concerning this specific point?
We are a bit far from the original issue :)

Pascal

Original comment by pascal.v...@gmail.com on 17 Dec 2010 at 7:29

• Changed state: Verified

[GoogleCodeExporter]

Hehe true :)

Maybe we should also make an issue concerning Caching the DB calls

Henry

Original comment by spreiter...@gmail.com on 17 Dec 2010 at 7:40

[GoogleCodeExporter]

I create a new issue for the annotation trigger.

Do you want me to add you as a committer to this little project?
As you want...

regards
Pascal

Original comment by pascal.v...@gmail.com on 17 Dec 2010 at 7:46

[GoogleCodeExporter]

Yes that would be nice, so I can commit my changes directly.

thanks
Henry

Original comment by spreiter...@gmail.com on 17 Dec 2010 at 7:48

[GoogleCodeExporter]

Just let you know that I didn't forget about the issue. I'm getting married in 
January so I don't have a lot of time for coding right now ;-)

Original comment by spreiter...@gmail.com on 20 Dec 2010 at 8:39

[GoogleCodeExporter]

Don't worry! I'm also very busy nowadays. Lots of work in my job before 
Christmas...
I'm also working on Siena in background because we are refactoring the trunk to 
make everything easier and more robust and migrating from googlecode to github 
and I'm enhancing the maven and GAE side of Siena...
And I wish you a happy marriage in the snow if you are in Europe ;)

Original comment by pascal.v...@gmail.com on 20 Dec 2010 at 9:36

[GoogleCodeExporter]

Thanks :-) I'm from Switzerland but I'm going to marry in Thailand :-)

Original comment by spreiter...@gmail.com on 20 Dec 2010 at 9:42