FSA Add WindowsAclFileAttributeViewsTest (Part 2)

This change adds support for faking Windows JNA calls
and uses faked JNA interfaces to test the
WindowsAclFileAttributeViews.getAclViews() method.

TODO: Add tests for Share ACL, and DFS ACLs.

[Brett, 2014-03-28 23:07:27]

Untabify JNA stubs

[Brett, 2014-03-28 23:38:34]

Fix 80 cols in UnsupportedKernel32

[pjo, 2014-03-28 23:41:32]

Adding John Felton to code review.

[mifern, 2014-04-01 03:03:17]

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/UnsupportedKernel32.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/UnsupportedKernel32.java:372:
DWORDByReference returnLength) {
Indentation is off.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViewsTest.java
(right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViewsTest.java:328:
.flags(FILE_INHERIT, DIRECTORY_INHERIT),
You should also add an entry with an empty flags. No flags would make it a
direct ACE.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViewsTest.java:423:
public void testGetAclViewsException1() throws Exception {
It's not clear what the purpose of testGetAclViewsException1 and
testGetAclViewsException2 are. Add comments to clarify what is being tested.

[Brett, 2014-04-01 17:47:01]

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/UnsupportedKernel32.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/UnsupportedKernel32.java:372:
DWORDByReference returnLength) {
On 2014/04/01 03:03:17, mifern wrote:
> Indentation is off.

Done.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViewsTest.java
(right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViewsTest.java:328:
.flags(FILE_INHERIT, DIRECTORY_INHERIT),
On 2014/04/01 03:03:17, mifern wrote:
> You should also add an entry with an empty flags. No flags would make it a
> direct ACE.

The expected direct AclEntries are constructed 3 lines down.  And in the
construction of the Win32 ACEs 8 lines below, it is the absence of
WinNT.INHERITED_ACE that makes one direct.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViewsTest.java:423:
public void testGetAclViewsException1() throws Exception {
On 2014/04/01 03:03:17, mifern wrote:
> It's not clear what the purpose of testGetAclViewsException1 and
> testGetAclViewsException2 are. Add comments to clarify what is being tested.

Done.  They are testing the first and second IOExceptions that can get thrown
out of WindowsAclFileAttributeViews.getFileSecurity().  I added comments.

[pjo, 2014-04-02 21:46:31]

Thank you.  Couple quick comments.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/AclView.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/AclView.java:198: if (!(obj instanceof
GroupPrincipal)) {
why isn't this a check for being an instanceof Group?

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/UnsupportedAdvapi32.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/UnsupportedAdvapi32.java:42: *
UnsupportedOperationException for everything.
Pls annotate use.

It's not intuitive what the use of an impl 
that throws UnsupportedOpEx for everything is.

Is it subclassing it?

[Brett, 2014-04-02 22:27:11]

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/AclView.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/AclView.java:198: if (!(obj instanceof
GroupPrincipal)) {
On 2014/04/02 21:46:31, pjo wrote:
> why isn't this a check for being an instanceof Group?

Because the production code creates instances of a different implementation of
the interface.  I *could* expose WindowsAclFileAttributeViews.User and Group
classes and add equals() to them.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/UnsupportedAdvapi32.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/UnsupportedAdvapi32.java:42: *
UnsupportedOperationException for everything.
On 2014/04/02 21:46:31, pjo wrote:
> Pls annotate use.
> 
> It's not intuitive what the use of an impl 
> that throws UnsupportedOpEx for everything is.
> 
> Is it subclassing it?

Done.  Added "Tests may subclass this and override only those methods used by
the object under test."
BTW, I hate the names of these classes, but modeled them after
UnsupportedDocIdPusher.

[pjo, 2014-04-03 00:02:54]

LGTM.  Thank you.

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/AclView.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/AclView.java:160: protected final String
name;
I guess protected is fine cause it is private class.
Curious why not use getName()?

[Brett, 2014-04-03 07:52:58]

Committed 02 April 2014 to File System Adaptor:
commit: f980661e7d7620e78dd71079a8c0c025072c63ad

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
File test/com/google/enterprise/adaptor/fs/AclView.java (right):

https://codereview.appspot.com/82070043/diff/30001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/fs/AclView.java:160: protected final String
name;
On 2014/04/03 00:02:54, pjo wrote:
> I guess protected is fine cause it is private class.
> Curious why not use getName()?

I could do that too.