Gilad Chaplik has posted comments on this change.

Change subject: core: extend DiskProfile.getAllForStorageDomain to a user query
......................................................................


Patch Set 2:

(6 comments)

new patch to follow

http://gerrit.ovirt.org/#/c/29812/2//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2014-07-14 20:14:13 +0300
Line 6: 
Line 7: core: extend DiskProfile.getAllForStorageDomain to a user query
Line 8: 
Line 9: Will be used for UP as well.
> s/UP/user portal
Done
Line 10: 
Line 11: Currenly we rely on SD permissions (no change from current flow).
Line 12: 
Line 13: Change-Id: I3bb75728cd76bba7e39de2314275f5bc87942c6b


Line 7: core: extend DiskProfile.getAllForStorageDomain to a user query
Line 8: 
Line 9: Will be used for UP as well.
Line 10: 
Line 11: Currenly we rely on SD permissions (no change from current flow).
> can you elaborate more? it's not well understood for someone who isn't that
Done
Line 12: 
Line 13: Change-Id: I3bb75728cd76bba7e39de2314275f5bc87942c6b


http://gerrit.ovirt.org/#/c/29812/2/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/profiles/DiskProfileDao.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/profiles/DiskProfileDao.java:

Line 23:      * @param storageDomainId
Line 24:      *            the storage domain's ID
Line 25:      * @param userId
Line 26:      *            the user's ID
Line 27:      * @param isFiltered
> please document this one as well or remove.
Done
Line 28:      * @return the list of disk profiles
Line 29:      */
Line 30:     List<DiskProfile> getAllForStorageDomain(Guid storageDomainId, 
Guid userId, boolean isFiltered);
Line 31: 


http://gerrit.ovirt.org/#/c/29812/2/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/profiles/DiskProfileDaoTest.java
File 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/profiles/DiskProfileDaoTest.java:

Line 50: 
Line 51:         assertNotNull(result);
Line 52:         assertEquals(FixturesTool.DISK_PROFILE_1, result.getId());
Line 53:     }
Line 54: 
> what about test to the added stored procedure?
Done
Line 55:     /**
Line 56:      * Ensures that an empty collection is returned.
Line 57:      */
Line 58:     @Test


Line 56:      * Ensures that an empty collection is returned.
Line 57:      */
Line 58:     @Test
Line 59:     public void testGetAllForStorageEmpty() {
Line 60:         List<DiskProfile> result = 
dao.getAllForStorageDomain(Guid.newGuid(), null, false);
> this line can remain the same, as we still have getAllForStorageDomain
Done
Line 61: 
Line 62:         assertNotNull(result);
Line 63:         assertTrue(result.isEmpty());
Line 64:     }


Line 67:      * Ensures that profiles are returned.
Line 68:      */
Line 69:     @Test
Line 70:     public void testGetAllForStorageDomainFull() {
Line 71:         List<DiskProfile> result = 
dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_SCALE_SD5, null, false);
> same
Done
Line 72: 
Line 73:         assertNotNull(result);
Line 74:         assertEquals(2, result.size());
Line 75:         for (DiskProfile diskProfile : result) {


-- 
To view, visit http://gerrit.ovirt.org/29812
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bb75728cd76bba7e39de2314275f5bc87942c6b
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to