[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/1350


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-216423449
  
@swill yes that's what I meant, therefore commented that it's ready for 
merge. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-216308956
  
@rafaelweingartner thanks for the review.  :)  I think what @rhtyd meant is 
anything outstanding is only cosmetic, I don't think he meant the entire 
discussion.

I think this one is ready to merge...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-216307184
  
@swill, 
I did a review on the comments and discussions to get me up to date (we had 
a discussion about some code duplications). Those discussions were pretty good, 
I believe that there was some block of code being repeated, and also some 
blocks (or lines I do not recall) that were never used or not needed.

I did a code walkthrough and I cannot say anything about the code. It is 
very good now. I can for sure give an LGMT here ;)

Sorry guys to say this, but, those discussions were not merely about 
cosmetics!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-216300372
  
@rafaelweingartner This one seems to be in OK shape.  I am missing one code 
review, given that you have actively reviewed this code, can I get your final 
status?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-216221000
  
LGTM, there are some comments on cosmetics but in general we should get 
this CI tested and merged cc @swill 

tag:easypr


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[bvbharatk]

Github user bvbharatk commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-200186297
  
### ACS CI BVT Run
 **Sumarry:**
 Build Number 121
 Hypervisor xenserver
 NetworkType Advanced
 Passed=104
 Failed=14
 Skipped=4

_Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0


**Failed tests:**
* :setup Failed

* integration.smoke.test_privategw_acl.TestPrivateGwACL

 * test_01_vpc_privategw_acl Failed


**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_deploy_vgpu_enabled_vm
test_06_copy_template
test_06_copy_iso

**Passed test suits:**
integration.smoke.test_deploy_vm_with_userdata.TestDeployVmWithUserData

integration.smoke.test_affinity_groups_projects.TestDeployVmWithAffinityGroup
integration.smoke.test_portable_publicip.TestPortablePublicIPAcquire
integration.smoke.test_over_provisioning.TestUpdateOverProvision
integration.smoke.test_global_settings.TestUpdateConfigWithScope
integration.smoke.test_scale_vm.TestScaleVm
integration.smoke.test_service_offerings.TestCreateServiceOffering
integration.smoke.test_loadbalance.TestLoadBalance
integration.smoke.test_routers.TestRouterServices
integration.smoke.test_reset_vm_on_reboot.TestResetVmOnReboot
integration.smoke.test_snapshots.TestSnapshotRootDisk

integration.smoke.test_deploy_vms_with_varied_deploymentplanners.TestDeployVmWithVariedPlanners
integration.smoke.test_network.TestDeleteAccount
integration.smoke.test_non_contigiousvlan.TestUpdatePhysicalNetwork
integration.smoke.test_deploy_vm_iso.TestDeployVMFromISO
integration.smoke.test_public_ip_range.TestDedicatePublicIPRange
integration.smoke.test_multipleips_per_nic.TestDeployVM
integration.smoke.test_regions.TestRegions
integration.smoke.test_affinity_groups.TestDeployVmWithAffinityGroup
integration.smoke.test_network_acl.TestNetworkACL
integration.smoke.test_pvlan.TestPVLAN
integration.smoke.test_volumes.TestCreateVolume
integration.smoke.test_ssvm.TestSSVMs
integration.smoke.test_nic.TestNic
integration.smoke.test_deploy_vm_root_resize.TestDeployVM
integration.smoke.test_resource_detail.TestResourceDetail
integration.smoke.test_secondary_storage.TestSecStorageServices
integration.smoke.test_vm_life_cycle.TestDeployVM
integration.smoke.test_disk_offerings.TestCreateDiskOffering


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-176082402
  
@DaanHoogland I agree there should be some guideline on code style. If 
these are there I think most of us will be willing to follow these. I guess it 
will not be difficult to adapt something that is already out there. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-176080946
  
@agneya2001 Sounds like you want to set the style guide. you and 
@rafaelweingartner should take this discussion to a higher level and take it to 
a discuss thread.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175931632
  
This code that was suggested:

private AccountVO createAccountVoForType(short type) {
AccountVO accountVO = new AccountVO();
accountVO.setType(type);
return accountVO;
  }

   @Test
public void testAdminLockableAccount() {
AccountVO accountVO = 
createAccountVoForType(Account.ACCOUNT_TYPE_ADMIN);
assertFalse(quotaManager.isLockable(accountVO));
}

@Test
public void testAdminLockableAccount() {
AccountVO accountVO = 
createAccountVoForType(Account.ACCOUNT_TYPE_ADMIN);
assertFalse(quotaManager.isLockable(accountVO));
}

and this code that will otherwise be:


   @Test
   public void testResourceDomainAdminLockableAccount() {
   AccountVO accountVO = new AccountVO();
accountVO.setType(Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN);
 assertFalse(quotaManager.isLockable(accountVO));
   }
   
   @Test
public void testProjectLockableAccount() {
AccountVO accountVO = new AccountVO();
accountVO.setType(Account.ACCOUNT_TYPE_PROJECT);
assertFalse(quotaManager.isLockable(accountVO));
   }
   

@rafaelweingartner  To me the code below looks simple and more readable. 
(In general if this was not the test code some of this code will be in 
constructor. Coming to constructors, there is much more repeatable code there 
in many cloudstack classes.) Now this might not look good to you and some 
others too. I suggest if that is the case take some effort and put up a code 
style guideline or at least adopt a guideline that already exists so that we 
donot have to go thru this. This will also be food for further cleanup as 
findbug and code style check already are CC: @bhaisaab @remibergsma 
@DaanHoogland @cristofolini. Till then this stands as it is.

 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-175472278
  
This patch cleans up any find bug issues in cloud-framework-quota and 
cloud-plugin-database-quota, verifiy output below. I think we should not delay 
this @remibergsma @DaanHoogland @bhaisaab @rafaelweingartner 



mvn -P enablefindbugs -pl :cloud-framework-quota verify
...

[INFO] --- maven-compiler-plugin:3.2:compile (default-compile) @ 
cloud-framework-quota ---
[INFO] Nothing to compile - all classes are up to date
[INFO] 
[INFO] >>> findbugs-maven-plugin:3.0.1:check (cloudstack-findbugs) > 
:findbugs @ cloud-framework-quota >>>
[INFO] 
[INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ 
cloud-framework-quota ---
[INFO] Fork Value is true
[INFO] Done FindBugs Analysis
[INFO] 
[INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack-findbugs) < 
:findbugs @ cloud-framework-quota <<<
[INFO] 
[INFO] --- findbugs-maven-plugin:3.0.1:check (cloudstack-findbugs) @ 
cloud-framework-quota ---
[INFO] BugInstance size is 0
[INFO] Error size is 0
[INFO] No errors/warnings found
[INFO] 


mvn -P enablefindbugs -pl :cloud-plugin-database-quota verify

..
[INFO] >>> findbugs-maven-plugin:3.0.1:check (cloudstack-findbugs) > 
:findbugs @ cloud-plugin-database-quota >>>
[INFO] 
[INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ 
cloud-plugin-database-quota ---
[INFO] Fork Value is true
[INFO] Done FindBugs Analysis
[INFO] 
[INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack-findbugs) < 
:findbugs @ cloud-plugin-database-quota <<<
[INFO] 
[INFO] --- findbugs-maven-plugin:3.0.1:check (cloudstack-findbugs) @ 
cloud-plugin-database-quota ---
[INFO] BugInstance size is 0
[INFO] Error size is 0
[INFO] No errors/warnings found



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175702905
  
@bhaisaab I think the test cases in different methods, each one for a 
single and unit test is the right way to go.

What I disagree is with the creation of “AccountVO” object. Anyone that 
looks at lines 202 – 204, can see that they are the same as 211- 214. 
Additionally, those "Id" and "DomainId" parameters that are being set are not 
used by the “isLockable” method. 

On top of everything, can you please look at lines 221-222, 231-232 and so 
forth; that is a clear example of code duplication without even thinking.
```
AccountVO accountVO = new AccountVO(); 
accountVO = new AccountVO();
```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1350#discussion_r51005837
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaManagerImplTest.java ---
@@ -197,4 +197,58 @@ public void testProcessQuotaBalanceForAccount() {
 Mockito.verify(quotaAcc, 
Mockito.times(1)).persistQuotaAccount(Mockito.any(QuotaAccountVO.class));
 }
 
+@Test
+public void testAdminLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_ADMIN);
+assertFalse(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testNormalLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(3L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+assertTrue(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void tesDomainAdmingLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(5L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_DOMAIN_ADMIN);
+assertTrue(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testReadOnlyAdminLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(6L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_READ_ONLY_ADMIN);
+assertFalse(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testResourceDomainAdminLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(7L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN);
+assertFalse(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testProjectLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(7L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_PROJECT);
+assertFalse(quotaManager.isLockable(accountVO));
--- End diff --

@abhinandanprateek We can actually remove the setId and setDomainId in all 
the testxxxLockableAccounts; create like a single class leve AccountVO and then 
in each of the testxxxLockableAccounts methods simply call the setType method 
and then call assert on them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1350#discussion_r51006727
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaManagerImplTest.java ---
@@ -197,4 +197,58 @@ public void testProcessQuotaBalanceForAccount() {
 Mockito.verify(quotaAcc, 
Mockito.times(1)).persistQuotaAccount(Mockito.any(QuotaAccountVO.class));
 }
 
+@Test
+public void testAdminLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_ADMIN);
+assertFalse(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testNormalLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(3L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+assertTrue(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void tesDomainAdmingLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(5L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_DOMAIN_ADMIN);
+assertTrue(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testReadOnlyAdminLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(6L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_READ_ONLY_ADMIN);
+assertFalse(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testResourceDomainAdminLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(7L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN);
+assertFalse(quotaManager.isLockable(accountVO));
+}
+
+@Test
+public void testProjectLockableAccount() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(7L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_PROJECT);
+assertFalse(quotaManager.isLockable(accountVO));
--- End diff --

@bhaisaab I don't agree with the change as it impacts simplicity and 
quality. All the three lines that you are asking me have a data element that 
will become a function parameter, I am not sure how it will simplify or result 
in enhanced quality.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175693496
  
@rafaelweingartner fair points, but it would make sense if you can comment 
on the diff in the context of the change.

Specific to the PR I see several testXXXLockableAccount which tries to test 
account types separately. Are you suggesting to get them all refactored, say 
put in a single testLockableAccount() and use a single Account/User object to 
test the lockability instead of explicitly separate test methods?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175708121
  
@agneya2001 how about you have a look at the test tomorrow and see if you 
can incorporate some of the suggestions like pull out the account from the 
method as a class variable?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175710627
  
@rafaelweingartner @bhaisaab I have taken out the duplicate account 
initialisation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175712977
  
@cristofolini your changes are incorporated can you review these.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175816413
  
I disagree that it would make the code more complex.
That is what I meant:
```
private AccountVO createAccountVoForType(short type) {
AccountVO accountVO = new AccountVO();
accountVO.setType(type);
return accountVO;
  }

   @Test
public void testAdminLockableAccount() {
AccountVO accountVO = 
createAccountVoForType(Account.ACCOUNT_TYPE_ADMIN);
assertFalse(quotaManager.isLockable(accountVO));
}
...
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175563484
  
@agneya2001 LGTM again, need one more review to get this merged along with 
some testing (though in this case, we've some unit tests for the change)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-175479331
  
LGTM @agneya2001 master seems to be unfrozen you'll need one more review to 
merge it yourself


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/1289


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-175500735
  
merging LGTM from @DaanHoogland and @bhaisaab 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175656788
  
@rafaelweingartner  It is only about realistic test data that is being 
passed to the method tested. You should differentiate between the tested code 
and the code that is testing. The logic/code that is being tested should have 
all the characteristics that you are talking about ie any data that is not 
being used should not be there.
But when it comes to writing tests and creating mocks you do it in a more 
realistic manner without deducing what could be happening inside the method as 
you only test what the method is supposed to be doing. That makes it error 
proof in future and that was what i was talking about.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175634392
  
@rafaelweingartner If it is a black box test then the person testing will 
obviously try to pass a realistic data set, that is what I have done. It is 
possible that tomorrow there are some changes to the tested method and the data 
that looks superficial may come into play and expose some erratic behaviour. 
The dataset passed mocks the dataset that will go thru the method in real 
runtime condition. Now offcourse we can see that the data is not being used in 
this method but does that mean I should change the dataset that mocks the real 
value ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175637289
  
If in a remotely distant future someone that we do not even know yet 
changes that method and make use of some extra data (forgetting to check the 
test cases), the test cases should catch that and then the person that is doing 
that change should fix the tests. That is why we write test cases, for the code 
of today, not for the code of tomorrow.

IMO we should prepare the architecture of software to the future, but the 
code itself should not be developed waiting for some futuristic hypothetical 
condition that may not ever happen.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175599430
  
Hi @agneya2001, there are a lot of duplicated lines at 
“QuotaManagerImplTest”, would you mind removing them?

Additionally, the lines you use to set the domainId and the id at the 
AccountVO object, you do not need to execute them, the method that you want to 
test does not need them.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175630573
  
It is cultural; we have to stop duplicating lines; no matter where and no 
matter how simple those blocks may seem. 

We also have to stop coding things that are not needed as you did in the 
test. If a code is not needed, why bother writing it down? I am referring to 
the “accountVO.setId” and accountVO.setDomainId” at the tests you wrote. 

I do not know if ACS has a guideline for that, for me, it looks like common 
sense. That was my opinion, but if others here are ok with the duplication of 
lines, feel free to merge the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175609511
  
@agneya2001  that is exactly what I meant. 
I disagree with you, letting this as they are, it is not simple. Why 
duplicate code, when we can do better? Especially if we use a good and 
descriptive method name.

About the initialization, I also disagree with you, at that point those 
values are not used, so they are needed and you do not need to code those lines.
If those attributes are needed somewhere else, they should be used only at 
the point in which they are needed.

Please, keep in mind the less the better. We need quality in our code, not 
quantity.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175624327
  
@rafaelweingartner quality is what we are discussing and I simply do not 
think that putting 3 lines of object initialisation in a method will make it 
high quality. Are you referring to some guidelines that I can see too ? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175601093
  
@rafaelweingartner Can you comment on the duplicated lines. There is a way 
you can get to the code and comment there.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175601814
  
I know that. I just did not want to go one by one and tell you that they 
are duplicated, I believe you can easily see that. Those duplicated lines I 
mentioned are in the test methods you created.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175603740
  
@rafaelweingartner if you mean initialisation of AccountVO to be moved into 
a method, and replacing it with a method call. I guess I would prefer it this 
way as this keeps things simple. 
The initialisation for domainId and id is done as the method will receive 
similar object, so no harm in passing a object that is close to the one passed 
at runtime, though as you said the function tested is not using it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175667113
  
I do not agree.
If people are comfortable with it, so let it be.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-175408926
  
@cristofolini @bhaisaab @remibergsma test code split into simpler methods.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[cristofolini]

Github user cristofolini commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-174211441
  
I see you've put a bunch of different test cases into a single 
`testLockableAccount` method. I'd strongly suggest you put each of those 
different cases into their own methods, so that if one of them fails in a 
build, we can easily determine what is failing. As it stands, that single test 
would fail if any of those cases goes wrong, leaving us to guess what failed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: consolidated lockable account chec...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1350#issuecomment-173218775
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-172558281
  
again code LGTM. Let's quickly run it 3 out of 5 findbugs issues will be 
fixed in this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-172563054
  
What needs to be done to fix them all?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-172594983
  
@remibergsma I would have to dive into my original investigation again. I 
am not sure who or where the others where created anymore. lat me markmail that 
for you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-172597438
  
here is my original request to look at them:
http://markmail.org/message/fqv252o3p7hmzwxz


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-172418090
  
@remibergsma @bhaisaab made review changes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-172197923
  
Ping @agneya2001 let's get this resolved soon :-) All those build failure 
mails hurt my eyes..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1289#discussion_r49316008
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java ---
@@ -310,14 +269,12 @@ public String getDeploymentPlanner() {
 }
 
 public String getDetail(String name) {
-assert (details != null) : "Did you forget to load the details?";
-
-return details != null ? details.get(name) : null;
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
+return details.get(name);
 }
 
 public void setDetail(String name, String value) {
-assert (details != null) : "Did you forget to load the details?";
-
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
--- End diff --

We can check that, but if we will let it as it is. 
 I think it should have a test case written to check that behavior.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1289#discussion_r49295533
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java ---
@@ -310,14 +269,12 @@ public String getDeploymentPlanner() {
 }
 
 public String getDetail(String name) {
-assert (details != null) : "Did you forget to load the details?";
-
-return details != null ? details.get(name) : null;
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
+return details.get(name);
 }
 
 public void setDetail(String name, String value) {
-assert (details != null) : "Did you forget to load the details?";
-
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
--- End diff --

I think that would work (returning an empty map) though we need to check 
the history as to why we never did that earlier. For now, the fix LGTM as it's 
better than the assert. We can take the hashmap fix separately.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-170455889
  
@agneya2001 can you help fix the issues, before the freeze. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1289#discussion_r49101315
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java ---
@@ -310,14 +269,12 @@ public String getDeploymentPlanner() {
 }
 
 public String getDetail(String name) {
-assert (details != null) : "Did you forget to load the details?";
-
-return details != null ? details.get(name) : null;
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
+return details.get(name);
 }
 
 public void setDetail(String name, String value) {
--- End diff --

what about a change in this method name?
I think "addDetail" would be better.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-169753995
  
Would you mind changing the comments over attributes “details” and 
“isDynamic” to proper java doc style? Additionally, what about changing 
them to private?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-16848
  
Ping @bhaisaab @agneya2001 the quota plugin unit tests are failing in `4.7` 
and `master` branches:

```
[INFO] 

[INFO] BUILD FAILURE
[INFO] 

[INFO] Total time: 02:44 min (Wall Clock)
[INFO] Finished at: 2016-01-03T09:51:53+01:00
[INFO] Final Memory: 78M/1371M
[INFO] 

[ERROR] Failed to execute goal 
org.apache.maven.plugins:maven-surefire-plugin:2.18.1:test (default-test) on 
project cloud-framework-quota: There are test failures.
[ERROR] 
```

Due to:
```
Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.054 sec
<<< FAILURE! - in org.apache.cloudstack.quota.QuotaStatementTest

testSendStatement(org.apache.cloudstack.quota.QuotaStatementTest)  Time
elapsed: 0.011 sec  <<< FAILURE!

org.mockito.exceptions.verification.WantedButNotInvoked:

Wanted but not invoked:

alertManager.sendQuotaAlert(



org.apache.cloudstack.quota.QuotaAlertManagerImpl$DeferredQuotaEmail@6e9d6ff7

);

-> at

org.apache.cloudstack.quota.QuotaStatementTest.testSendStatement(QuotaStatementTest.java:251)

Actually, there were zero interactions with this mock.


at

org.apache.cloudstack.quota.QuotaStatementTest.testSendStatement(QuotaStatementTest.java:251)



Results :


Failed tests:

  QuotaStatementTest.testSendStatement:251

Wanted but not invoked:

alertManager.sendQuotaAlert(



org.apache.cloudstack.quota.QuotaAlertManagerImpl$DeferredQuotaEmail@6e9d6ff7

);

-> at

org.apache.cloudstack.quota.QuotaStatementTest.testSendStatement(QuotaStatementTest.java:251)
```

Please fix against `4.7` branch after which we will forward merge to 
`master`. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[agneya2001]

GitHub user agneya2001 opened a pull request:

https://github.com/apache/cloudstack/pull/1289

Quota: findbug fixes

Findbug fixes for cloud-framework-quota and cloud-plugin-database-quota.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shapeblue/cloudstack master-quotafb

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1289.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1289


commit 625abb80aa590cfc7e6cfefe6f0bf55c95ac679f
Author: Abhinandan Prateek 
Date:   2015-12-23T04:42:11Z

Quota: findbug fixes




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-167554252
  
Based on the code LGTM, tests running


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1289#discussion_r48476072
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java ---
@@ -310,14 +269,12 @@ public String getDeploymentPlanner() {
 }
 
 public String getDetail(String name) {
-assert (details != null) : "Did you forget to load the details?";
-
-return details != null ? details.get(name) : null;
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
+return details.get(name);
 }
 
 public void setDetail(String name, String value) {
-assert (details != null) : "Did you forget to load the details?";
-
+if (details == null) throw new IllegalStateException("Cannot set 
value as details is null");
--- End diff --

good solution :+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1289#discussion_r48474787
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java ---
@@ -312,7 +271,7 @@ public String getDeploymentPlanner() {
 public String getDetail(String name) {
 assert (details != null) : "Did you forget to load the details?";
--- End diff --

I would have solved this one by removing the assert. Do we even run any 
tests with asserts enabled? (no :-1: intended!)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota: findbug fixes

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1289#issuecomment-167625390
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1205#discussion_r47335119
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
 ---
@@ -233,9 +233,9 @@ public int compare(QuotaBalanceVO o1, QuotaBalanceVO 
o2) {
 // order is desc last item is the start item
 QuotaBalanceVO startItem = 
quotaBalance.get(quotaBalance.size() - 1);
 QuotaBalanceVO endItem = quotaBalance.get(0);
-resp.setStartDate(startItem.getUpdatedOn());
+resp.setStartDate(startDate);
--- End diff --

LGTM then :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1205#issuecomment-163885070
  
Alright, re-review it. LGTM.
cc @remibergsma @DaanHoogland 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1205#issuecomment-163882122
  
LGTM (tested on Abhi's env)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1205#discussion_r47333902
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaBalanceCmd.java
 ---
@@ -83,7 +83,12 @@ public void setDomainId(Long domainId) {
 }
 
 public Date getEndDate() {
-return endDate == null ? null : 
_responseBuilder.startOfNextDay(endDate == null ? null : new 
Date(endDate.getTime()));
+if (endDate==null){
--- End diff --

minor style issue: add a space between comparators so like
 if (endData == null)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1205#discussion_r47333986
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
 ---
@@ -233,9 +233,9 @@ public int compare(QuotaBalanceVO o1, QuotaBalanceVO 
o2) {
 // order is desc last item is the start item
 QuotaBalanceVO startItem = 
quotaBalance.get(quotaBalance.size() - 1);
 QuotaBalanceVO endItem = quotaBalance.get(0);
-resp.setStartDate(startItem.getUpdatedOn());
+resp.setStartDate(startDate);
--- End diff --

instead of startDate, pass a new object like new Date(startDate.getTime()) 
for the ref issue we discovered with the earlier PR. Alternatively, you may fix 
the setters and getters to do return or set using new Date() syntax


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[agneya2001]

Github user agneya2001 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1205#discussion_r47334512
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
 ---
@@ -233,9 +233,9 @@ public int compare(QuotaBalanceVO o1, QuotaBalanceVO 
o2) {
 // order is desc last item is the start item
 QuotaBalanceVO startItem = 
quotaBalance.get(quotaBalance.size() - 1);
 QuotaBalanceVO endItem = quotaBalance.get(0);
-resp.setStartDate(startItem.getUpdatedOn());
+resp.setStartDate(startDate);
--- End diff --

setStartDate of QuotaBalanceResponse makes a copy of the date passed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: Ensuring that the dates displayed ...

[agneya2001]

GitHub user agneya2001 opened a pull request:

https://github.com/apache/cloudstack/pull/1205

QUOTA: Ensuring that the dates displayed are as per user expectations

When querying db we use start of next day to query quota usage for
today, but while displaying it to user we still need to show it as
todays date

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shapeblue/cloudstack master-9126

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1205.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1205


commit 0bad8dd18a445d9570bf6589bf9b5bbeccbd4dca
Author: Abhinandan Prateek 
Date:   2015-12-08T04:29:06Z

QUOTA: Ensuring that the dates displayed are as per user expectations

When querying db we use start of next day to query quota usage for
today, but while displaying it to user we still need to show it as
todays date




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: ENSURING THAT THE DATES DISPLAYED ...

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/1193#issuecomment-163479275
  
Closing this to change the case of the title.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: ENSURING THAT THE DATES DISPLAYED ...

[agneya2001]

Github user agneya2001 closed the pull request at:

https://github.com/apache/cloudstack/pull/1193


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: ENSURING THAT THE DATES DISPLAYED ...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1193#issuecomment-163330589
  
@agneya2001 Can you please change the title/commit to not only use capitals?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: QUOTA: ENSURING THAT THE DATES DISPLAYED ...

[agneya2001]

GitHub user agneya2001 opened a pull request:

https://github.com/apache/cloudstack/pull/1193

QUOTA: ENSURING THAT THE DATES DISPLAYED ARE AS PER USER EXPECTATIONS

When querying db we use start of next day to query quota usage for
today, but while displaying it to user we still need to show it as
todays date.

A regression due to feature review fixes is also made.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shapeblue/cloudstack master-9126

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1193.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1193


commit 5243ec882dc078edcbf10d5f0f618f7d9d9f5592
Author: Abhinandan Prateek 
Date:   2015-12-08T04:29:06Z

QUOTA: ENSURING THAT THE DATES DISPLAYED ARE AS PER USER EXPECTATIONS

When querying db we use start of next day to query quota usage for
today, but while displaying it to user we still need to show it as
todays date




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[agneya2001]

Github user agneya2001 commented on the pull request:

https://github.com/apache/cloudstack/pull/768#issuecomment-162529656
  
@bhaisaab @remibergsma there is still one issue that I am looking at now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/768#issuecomment-162572129
  
@agneya2001 OK, please ping me when done!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/768#issuecomment-162573927
  
@remibergsma sure, I'm discussing with Abhi on that minor fix. After that, 
will squash/amend the message in the git log.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/768#issuecomment-162572535
  
@bhaisaab Minor thing: could you also rename the title to "Implement Quota 
service"?

I will make sure it will be merged before we freeze.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[agneya2001]

Github user agneya2001 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46796541
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaAlertManager.stop());
+}
+
+@Test
+public void testCheckAndSendQuotaAlertEmails() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(accountVO);
+
+QuotaAccountVO acc = new 

[GitHub] cloudstack pull request: Quota

[agneya2001]

Github user agneya2001 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46796893
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaAlertManager.stop());
+}
+
+@Test
+public void testCheckAndSendQuotaAlertEmails() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(accountVO);
+
+QuotaAccountVO acc = new 

[GitHub] cloudstack pull request: Quota

[agneya2001]

Github user agneya2001 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46796854
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaManagerImplTest.java ---
@@ -0,0 +1,203 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.usage.UsageVO;
+import com.cloud.usage.dao.UsageDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaBalanceDao;
+import org.apache.cloudstack.quota.dao.QuotaTariffDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.dao.ServiceOfferingDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaTariffVO;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.usage.UsageTypes;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.ConfigurationException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaManagerImplTest extends TestCase {
+
+@Mock
+private AccountDao accountDao;
+@Mock
+private QuotaAccountDao quotaAcc;
+@Mock
+private UsageDao usageDao;
+@Mock
+private QuotaTariffDao quotaTariffDao;
+@Mock
+private QuotaUsageDao quotaUsageDao;
+@Mock
+private ServiceOfferingDao serviceOfferingDao;
+@Mock
+private QuotaBalanceDao quotaBalanceDao;
+@Mock
+private ConfigurationDao configDao;
+
+@Spy
+QuotaManagerImpl quotaManager = new QuotaManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(usageDao, "_usageDao");
+injectMockToField(quotaTariffDao, "_quotaTariffDao");
+injectMockToField(quotaUsageDao, "_quotaUsageDao");
+injectMockToField(serviceOfferingDao, "_serviceOfferingDao");
+injectMockToField(quotaBalanceDao, "_quotaBalanceDao");
+injectMockToField(configDao, "_configDao");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaManager.stop());
+}
+
+@Test
+public void testConfig() throws ConfigurationException {
+
Mockito.when(configDao.getConfiguration(Mockito.anyMapOf(String.class, 
Object.class))).thenReturn(new HashMap());
+Map map = new HashMap<>();
+map.put("usage.stats.job.aggregation.range", "0");
+   

[GitHub] cloudstack pull request: Quota

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46797933
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaAlertManager.stop());
+}
+
+@Test
+public void testCheckAndSendQuotaAlertEmails() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(accountVO);
+
+QuotaAccountVO acc = new 

[GitHub] cloudstack pull request: Quota

[bhaisaab]

Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46797960
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaManagerImplTest.java ---
@@ -0,0 +1,203 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.usage.UsageVO;
+import com.cloud.usage.dao.UsageDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaBalanceDao;
+import org.apache.cloudstack.quota.dao.QuotaTariffDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.dao.ServiceOfferingDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaTariffVO;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.usage.UsageTypes;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.ConfigurationException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaManagerImplTest extends TestCase {
+
+@Mock
+private AccountDao accountDao;
+@Mock
+private QuotaAccountDao quotaAcc;
+@Mock
+private UsageDao usageDao;
+@Mock
+private QuotaTariffDao quotaTariffDao;
+@Mock
+private QuotaUsageDao quotaUsageDao;
+@Mock
+private ServiceOfferingDao serviceOfferingDao;
+@Mock
+private QuotaBalanceDao quotaBalanceDao;
+@Mock
+private ConfigurationDao configDao;
+
+@Spy
+QuotaManagerImpl quotaManager = new QuotaManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(usageDao, "_usageDao");
+injectMockToField(quotaTariffDao, "_quotaTariffDao");
+injectMockToField(quotaUsageDao, "_quotaUsageDao");
+injectMockToField(serviceOfferingDao, "_serviceOfferingDao");
+injectMockToField(quotaBalanceDao, "_quotaBalanceDao");
+injectMockToField(configDao, "_configDao");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaManager.stop());
+}
+
+@Test
+public void testConfig() throws ConfigurationException {
+
Mockito.when(configDao.getConfiguration(Mockito.anyMapOf(String.class, 
Object.class))).thenReturn(new HashMap());
+Map map = new HashMap<>();
+map.put("usage.stats.job.aggregation.range", "0");
+ 

[GitHub] cloudstack pull request: Quota

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/768#issuecomment-162506613
  
Abhi has fixed the issues, changes  are now incorporated, commits squashed 
into one.
cc @remibergsma @jburwell

LGTM, let's merge this before EOD.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46783089
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/constant/QuotaConfig.java ---
@@ -0,0 +1,53 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+
+package org.apache.cloudstack.quota.constant;
+
+import org.apache.cloudstack.framework.config.ConfigKey;
+
+public interface QuotaConfig {
--- End diff --

@abhinandanprateek it is idiomatic to exclude the unnecessary access 
modifiers for maintainability reasons.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46783125
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/constant/QuotaConfig.java ---
@@ -0,0 +1,53 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+
+package org.apache.cloudstack.quota.constant;
+
+import org.apache.cloudstack.framework.config.ConfigKey;
+
+public interface QuotaConfig {
+
+public static final ConfigKey QuotaPluginEnabled = new 
ConfigKey("Advanced", Boolean.class, "quota.enable.service", "false", 
"Indicates whether Quota plugin is enabled or not",
+true);
+
+public static final ConfigKey QuotaEnableEnforcement = new 
ConfigKey("Advanced", String.class, "quota.enable.enforcement", "false",
+"Enable the usage quota enforcement, i.e. on true when 
exceeding quota the respective account will be locked.", true);
+
+public static final ConfigKey QuotaCurrencySymbol = new 
ConfigKey("Advanced", String.class, "quota.currency.symbol", "R", "The 
symbol for the currency in use to measure usage.",
+true);
+
+public static final ConfigKey QuotaSmtpHost = new 
ConfigKey("Advanced", String.class, "quota.usage.smtp.host", "", "Quota 
SMTP host for quota related emails", true);
+
+public static final ConfigKey QuotaSmtpTimeout = new 
ConfigKey("Advanced", String.class, 
"quota.usage.smtp.connection.timeout", "60",
+"Quota SMTP server connection timeout duration", true);
+
+public static final ConfigKey QuotaSmtpUser = new 
ConfigKey("Advanced", String.class, "quota.usage.smtp.user", "", "Quota 
SMTP server username", true);
+
+public static final ConfigKey QuotaSmtpPassword = new 
ConfigKey("Advanced", String.class, "quota.usage.smtp.password", "", 
"Quota SMTP server password", true);
+
+public static final ConfigKey QuotaSmtpPort = new 
ConfigKey("Advanced", String.class, "quota.usage.smtp.port", "", "Quota 
SMTP port", true);
+
+public static final ConfigKey QuotaSmtpAuthType = new 
ConfigKey("Advanced", String.class, "quota.usage.smtp.useAuth", "",
+"If true, use secure SMTP authentication when sending 
emails.", true);
+
+public static final ConfigKey QuotaSmtpSender = new 
ConfigKey("Advanced", String.class, "quota.usage.smtp.sender", "",
+"Sender of quota alert email (will be in the From header of 
the email)", true);
+
--- End diff --

@abhinandanprateek why are they necessary?  Under what circumstances would 
a user have a separate mail server for quota alerts and all other CloudStack 
alerts?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784327
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaAlertManager.stop());
+}
+
+@Test
+public void testCheckAndSendQuotaAlertEmails() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(accountVO);
+
+QuotaAccountVO acc = new 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784536
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaBalanceCmd.java
 ---
@@ -0,0 +1,125 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.QuotaBalanceResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
+import org.apache.cloudstack.api.response.QuotaStatementItemResponse;
+
+@APICommand(name = "quotaBalance", responseObject = 
QuotaStatementItemResponse.class, description = "Create a quota balance 
statement", since = "4.6.0", requestHasSensitiveInfo = false, 
responseHasSensitiveInfo = false)
+public class QuotaBalanceCmd extends BaseCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(QuotaBalanceCmd.class);
+
+private static final String s_name = "quotabalanceresponse";
+
+@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, 
required = true, description = "Account Id for which statement needs to be 
generated")
+private String accountName;
+
+@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, 
required = true, entityType = DomainResponse.class, description = "If domain Id 
is given and the caller is domain admin then the statement is generated for 
domain.")
+private Long domainId;
+
+@Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, 
description = "End date range for quota query. Use -MM-dd as the date 
format, e.g. startDate=2009-06-03.")
+private Date endDate;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
description = "Start date range quota query. Use -MM-dd as the date format, 
e.g. startDate=2009-06-01.")
+private Date startDate;
+
+@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, 
entityType = AccountResponse.class, description = "List usage records for the 
specified account")
+private Long accountId;
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public Date getEndDate() {
+return endDate == null ? null : 
_responseBuilder.startOfNextDay(endDate);
--- End diff --

Since ``Date``s are mutable, ``endDate`` should be copied before being 
passed into ``_responseBuilder.startOfNextDay`` to avoid downstream 
side-effects.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784663
  
--- Diff: engine/schema/src/com/cloud/usage/UsageVO.java ---
@@ -328,4 +339,48 @@ public Date getStartDate() {
 public Date getEndDate() {
 return endDate;
 }
+
+public void setId(Long id) {
+this.id = id;
+}
+
+public void setType(String type) {
+this.type = type;
+}
+
+public void setStartDate(Date startDate) {
+this.startDate = startDate;
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
--- End diff --

Not part of quota changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784674
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaStatementCmd.java
 ---
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.api.response.QuotaStatementResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.api.response.QuotaStatementItemResponse;
+
+import com.cloud.user.Account;
+
+@APICommand(name = "quotaStatement", responseObject = 
QuotaStatementItemResponse.class, description = "Create a quota statement", 
since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = 
false)
+public class QuotaStatementCmd extends BaseCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(QuotaStatementCmd.class);
+
+private static final String s_name = "quotastatementresponse";
+
+@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, 
required = true, description = "Optional, Account Id for which statement needs 
to be generated")
+private String accountName;
+
+@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, 
required = true, entityType = DomainResponse.class, description = "Optional, If 
domain Id is given and the caller is domain admin then the statement is 
generated for domain.")
+private Long domainId;
+
+@Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, 
required = true, description = "End date range for quota query. Use -MM-dd 
as the date format, e.g. startDate=2009-06-03.")
+private Date endDate;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
required = true, description = "Start date range quota query. Use -MM-dd as 
the date format, e.g. startDate=2009-06-01.")
+private Date startDate;
+
+@Parameter(name = ApiConstants.TYPE, type = CommandType.INTEGER, 
description = "List quota usage records for the specified usage type")
+private Integer usageType;
+
+@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, 
entityType = AccountResponse.class, description = "List usage records for the 
specified account")
+private Long accountId;
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public Integer getUsageType() {
+return usageType;
+}
+
+public void setUsageType(Integer usageType) {
+this.usageType = usageType;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public Date getEndDate() {
+return _responseBuilder.startOfNextDay(endDate == null ? new 
Date() : endDate);
--- End diff --

Since ``Date``s are mutable, a copy of ``endDate`` should be passed into 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784639
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaStatementCmd.java
 ---
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.api.response.QuotaStatementResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.api.response.QuotaStatementItemResponse;
+
+import com.cloud.user.Account;
+
+@APICommand(name = "quotaStatement", responseObject = 
QuotaStatementItemResponse.class, description = "Create a quota statement", 
since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = 
false)
+public class QuotaStatementCmd extends BaseCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(QuotaStatementCmd.class);
+
+private static final String s_name = "quotastatementresponse";
+
+@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, 
required = true, description = "Optional, Account Id for which statement needs 
to be generated")
+private String accountName;
+
+@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, 
required = true, entityType = DomainResponse.class, description = "Optional, If 
domain Id is given and the caller is domain admin then the statement is 
generated for domain.")
+private Long domainId;
+
+@Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, 
required = true, description = "End date range for quota query. Use -MM-dd 
as the date format, e.g. startDate=2009-06-03.")
+private Date endDate;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
required = true, description = "Start date range quota query. Use -MM-dd as 
the date format, e.g. startDate=2009-06-01.")
+private Date startDate;
+
+@Parameter(name = ApiConstants.TYPE, type = CommandType.INTEGER, 
description = "List quota usage records for the specified usage type")
+private Integer usageType;
+
+@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, 
entityType = AccountResponse.class, description = "List usage records for the 
specified account")
+private Long accountId;
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public Integer getUsageType() {
+return usageType;
+}
+
+public void setUsageType(Integer usageType) {
+this.usageType = usageType;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public Date getEndDate() {
+return _responseBuilder.startOfNextDay(endDate == null ? new 
Date() : endDate);
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
+}
+
 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784756
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaTariffListCmd.java
 ---
@@ -0,0 +1,95 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import com.cloud.user.Account;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.api.response.QuotaTariffResponse;
+import org.apache.cloudstack.quota.vo.QuotaTariffVO;
+import org.apache.log4j.Logger;
+
+import javax.inject.Inject;
+
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@APICommand(name = "quotaTariffList", responseObject = 
QuotaTariffResponse.class, description = "Lists all quota tariff plans", since 
= "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class QuotaTariffListCmd extends BaseListCmd {
+public static final Logger s_logger = 
Logger.getLogger(QuotaTariffListCmd.class);
+private static final String s_name = "quotatarifflistresponse";
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+@Parameter(name = ApiConstants.USAGE_TYPE, type = CommandType.INTEGER, 
required = false, description = "Usage type of the resource")
+private Integer usageType;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
required = false, description = "The effective start date on/after which the 
quota tariff is effective and older tariffs are no longer used for the usage 
type. Use -MM-dd as the date format, e.g. startDate=2009-06-03.")
+private Date effectiveDate;
+
+public QuotaTariffListCmd() {
--- End diff --

In absence of any declared constructors, the compiler will generate a 
public, no arg constructor which will satisfy Spring.  This code is simply 
crufty boilerplate.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784709
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaStatementCmd.java
 ---
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.api.response.QuotaStatementResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.api.response.QuotaStatementItemResponse;
+
+import com.cloud.user.Account;
+
+@APICommand(name = "quotaStatement", responseObject = 
QuotaStatementItemResponse.class, description = "Create a quota statement", 
since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = 
false)
+public class QuotaStatementCmd extends BaseCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(QuotaStatementCmd.class);
+
+private static final String s_name = "quotastatementresponse";
+
+@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, 
required = true, description = "Optional, Account Id for which statement needs 
to be generated")
+private String accountName;
+
+@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, 
required = true, entityType = DomainResponse.class, description = "Optional, If 
domain Id is given and the caller is domain admin then the statement is 
generated for domain.")
+private Long domainId;
+
+@Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, 
required = true, description = "End date range for quota query. Use -MM-dd 
as the date format, e.g. startDate=2009-06-03.")
+private Date endDate;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
required = true, description = "Start date range quota query. Use -MM-dd as 
the date format, e.g. startDate=2009-06-01.")
+private Date startDate;
+
+@Parameter(name = ApiConstants.TYPE, type = CommandType.INTEGER, 
description = "List quota usage records for the specified usage type")
+private Integer usageType;
+
+@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, 
entityType = AccountResponse.class, description = "List usage records for the 
specified account")
+private Long accountId;
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public Integer getUsageType() {
+return usageType;
+}
+
+public void setUsageType(Integer usageType) {
+this.usageType = usageType;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public Date getEndDate() {
+return _responseBuilder.startOfNextDay(endDate == null ? new 
Date() : endDate);
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
+}
+
 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784117
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaStatementTest.java ---
@@ -0,0 +1,248 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.QuotaStatementImpl.STATEMENT_PERIODS;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.ConfigurationException;
+
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaStatementTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManager emailQuotaAlert;
+
+@Spy
+QuotaStatementImpl quotaStatement = new QuotaStatementImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaStatementImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaStatement, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaStatementImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_quotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaStatement.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaStatement.stop());
+}
+
+@Test
+public void testStatementPeriodBIMONTHLY() {
+Calendar date = Calendar.getInstance();
+
+//BIMONTHLY - first statement of month
+date.set(Calendar.DATE, 
QuotaStatementImpl.s_LAST_STATEMENT_SENT_DAYS + 1);
+Calendar period[] = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period == null);
+
+//1 of this month
+date.set(Calendar.DATE, 1);
+period = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period != null);
+assertTrue(period[0].toString(), period[0].before(period[1]));
+assertTrue(period[0].toString(), period[0].get(Calendar.DATE) == 
1);
+assertTrue(period[1].toString(), period[1].get(Calendar.DATE) == 
15);
+
+//BIMONTHLY - second statement of month
+date = Calendar.getInstance();
+date.set(Calendar.DATE, 
QuotaStatementImpl.s_LAST_STATEMENT_SENT_DAYS + 16);
+period = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period == null);
+
+//17 of this month

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784126
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaStatementTest.java ---
@@ -0,0 +1,248 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.QuotaStatementImpl.STATEMENT_PERIODS;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.ConfigurationException;
+
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaStatementTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManager emailQuotaAlert;
+
+@Spy
+QuotaStatementImpl quotaStatement = new QuotaStatementImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaStatementImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaStatement, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaStatementImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_quotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaStatement.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaStatement.stop());
+}
+
+@Test
+public void testStatementPeriodBIMONTHLY() {
+Calendar date = Calendar.getInstance();
+
+//BIMONTHLY - first statement of month
+date.set(Calendar.DATE, 
QuotaStatementImpl.s_LAST_STATEMENT_SENT_DAYS + 1);
+Calendar period[] = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period == null);
+
+//1 of this month
+date.set(Calendar.DATE, 1);
+period = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period != null);
+assertTrue(period[0].toString(), period[0].before(period[1]));
+assertTrue(period[0].toString(), period[0].get(Calendar.DATE) == 
1);
+assertTrue(period[1].toString(), period[1].get(Calendar.DATE) == 
15);
+
+//BIMONTHLY - second statement of month
+date = Calendar.getInstance();
+date.set(Calendar.DATE, 
QuotaStatementImpl.s_LAST_STATEMENT_SENT_DAYS + 16);
+period = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period == null);
+
+//17 of this month

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784145
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaStatementTest.java ---
@@ -0,0 +1,248 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.QuotaStatementImpl.STATEMENT_PERIODS;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.ConfigurationException;
+
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaStatementTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManager emailQuotaAlert;
+
+@Spy
+QuotaStatementImpl quotaStatement = new QuotaStatementImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaStatementImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaStatement, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaStatementImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_quotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaStatement.start(); // expected to fail as pid is not 
available
--- End diff --

If this is expected to fail the line 84 should a ``fail`` call since it is 
expected that this operation will throw an exception.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784388
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaAlertManager.stop());
+}
+
+@Test
+public void testCheckAndSendQuotaAlertEmails() {
+AccountVO accountVO = new AccountVO();
+accountVO.setId(2L);
+accountVO.setDomainId(1L);
+accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
+
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(accountVO);
+
+QuotaAccountVO acc = new 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784395
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
--- End diff --

If this is expected to fail the line 101 should call ``fail`` since it is 
expected that this operation will throw an exception.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784408
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaAlertManagerImplTest.java 
---
@@ -0,0 +1,205 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.mail.MessagingException;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaAlertManagerImplTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+UserDao userDao;
+@Mock
+DomainDao domainDao;
+@Mock
+QuotaEmailTemplatesDao quotaEmailTemplateDao;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManagerImpl.EmailQuotaAlert emailQuotaAlert;
+
+@Spy
+QuotaAlertManagerImpl quotaAlertManager = new QuotaAlertManagerImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaAlertManagerImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaAlertManager, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaAlertManagerImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(userDao, "_userDao");
+injectMockToField(domainDao, "_domainDao");
+injectMockToField(quotaEmailTemplateDao, "_quotaEmailTemplateDao");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_emailQuotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaAlertManager.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaAlertManager.stop());
--- End diff --

Why are we testing the ``stop`` operation when the test method expects the 
``start`` operation to fail?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46782245
  
--- Diff: 
framework/quota/src/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java ---
@@ -0,0 +1,408 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.Account.State;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+import com.sun.mail.smtp.SMTPMessage;
+import com.sun.mail.smtp.SMTPSSLTransport;
+import com.sun.mail.smtp.SMTPTransport;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.constant.QuotaConfig;
+import 
org.apache.cloudstack.quota.constant.QuotaConfig.QuotaEmailTemplateTypes;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
+import org.apache.commons.lang3.text.StrSubstitutor;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import javax.ejb.Local;
+import javax.inject.Inject;
+import javax.mail.Authenticator;
+import javax.mail.Message;
+import javax.mail.MessagingException;
+import javax.mail.PasswordAuthentication;
+import javax.mail.Session;
+import javax.mail.URLName;
+import javax.mail.internet.InternetAddress;
+import javax.naming.ConfigurationException;
+import java.io.UnsupportedEncodingException;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+@Component
+@Local(value = QuotaAlertManager.class)
+public class QuotaAlertManagerImpl extends ManagerBase implements 
QuotaAlertManager {
+private static final Logger s_logger = 
Logger.getLogger(QuotaAlertManagerImpl.class);
+
+@Inject
+private AccountDao _accountDao;
+@Inject
+private QuotaAccountDao _quotaAcc;
+@Inject
+private UserDao _userDao;
+@Inject
+private DomainDao _domainDao;
+@Inject
+private QuotaEmailTemplatesDao _quotaEmailTemplateDao;
+@Inject
+private ConfigurationDao _configDao;
+@Inject
+private QuotaUsageDao _quotaUsage;
+
+private EmailQuotaAlert _emailQuotaAlert;
+private boolean _lockAccountEnforcement = false;
+
+boolean _smtpDebug = false;
+
+public QuotaAlertManagerImpl() {
+super();
+}
+
+private void mergeConfigs(Map dbParams, Map xmlParams) {
+for (Map.Entry param : xmlParams.entrySet()) {
+dbParams.put(param.getKey(), (String)param.getValue());
+}
+}
+
+@Override
+public boolean configure(String name, Map params) 
throws ConfigurationException {
+super.configure(name, params);
+
+Map configs = _configDao.getConfiguration(params);
+
+if (params != null) {
+mergeConfigs(configs, params);
+}
+
+final String smtpHost = 
configs.get(QuotaConfig.QuotaSmtpHost.key());
+int smtpPort = 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784092
  
--- Diff: 
framework/quota/test/org/apache/cloudstack/quota/QuotaStatementTest.java ---
@@ -0,0 +1,248 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.quota;
+
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.db.TransactionLegacy;
+import junit.framework.TestCase;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.quota.QuotaStatementImpl.STATEMENT_PERIODS;
+import org.apache.cloudstack.quota.dao.QuotaAccountDao;
+import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.ConfigurationException;
+
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QuotaStatementTest extends TestCase {
+
+@Mock
+AccountDao accountDao;
+@Mock
+QuotaAccountDao quotaAcc;
+@Mock
+ConfigurationDao configDao;
+@Mock
+QuotaUsageDao quotaUsage;
+@Mock
+QuotaAlertManager emailQuotaAlert;
+
+@Spy
+QuotaStatementImpl quotaStatement = new QuotaStatementImpl();
+
+private void injectMockToField(Object mock, String fieldName) throws 
NoSuchFieldException, IllegalAccessException {
+Field f = QuotaStatementImpl.class.getDeclaredField(fieldName);
+f.setAccessible(true);
+f.set(quotaStatement, mock);
+}
+
+@Before
+public void setup() throws IllegalAccessException, 
NoSuchFieldException, ConfigurationException {
+// Dummy transaction stack setup
+TransactionLegacy.open("QuotaStatementImplTest");
+
+injectMockToField(accountDao, "_accountDao");
+injectMockToField(quotaAcc, "_quotaAcc");
+injectMockToField(configDao, "_configDao");
+injectMockToField(quotaUsage, "_quotaUsage");
+injectMockToField(emailQuotaAlert, "_quotaAlert");
+}
+
+@Test
+public void testStartStop() {
+try {
+quotaStatement.start(); // expected to fail as pid is not 
available
+} catch (NumberFormatException ignored) {
+}
+assertTrue(quotaStatement.stop());
+}
+
+@Test
+public void testStatementPeriodBIMONTHLY() {
+Calendar date = Calendar.getInstance();
+
+//BIMONTHLY - first statement of month
+date.set(Calendar.DATE, 
QuotaStatementImpl.s_LAST_STATEMENT_SENT_DAYS + 1);
+Calendar period[] = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period == null);
+
+//1 of this month
+date.set(Calendar.DATE, 1);
+period = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period != null);
+assertTrue(period[0].toString(), period[0].before(period[1]));
+assertTrue(period[0].toString(), period[0].get(Calendar.DATE) == 
1);
+assertTrue(period[1].toString(), period[1].get(Calendar.DATE) == 
15);
+
+//BIMONTHLY - second statement of month
+date = Calendar.getInstance();
+date.set(Calendar.DATE, 
QuotaStatementImpl.s_LAST_STATEMENT_SENT_DAYS + 16);
+period = quotaStatement.statementTime(date, 
STATEMENT_PERIODS.BIMONTHLY);
+assertTrue(period == null);
+
+//17 of this month

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784622
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaStatementCmd.java
 ---
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.api.response.QuotaStatementResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.api.response.QuotaStatementItemResponse;
+
+import com.cloud.user.Account;
+
+@APICommand(name = "quotaStatement", responseObject = 
QuotaStatementItemResponse.class, description = "Create a quota statement", 
since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = 
false)
+public class QuotaStatementCmd extends BaseCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(QuotaStatementCmd.class);
+
+private static final String s_name = "quotastatementresponse";
+
+@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, 
required = true, description = "Optional, Account Id for which statement needs 
to be generated")
+private String accountName;
+
+@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, 
required = true, entityType = DomainResponse.class, description = "Optional, If 
domain Id is given and the caller is domain admin then the statement is 
generated for domain.")
+private Long domainId;
+
+@Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, 
required = true, description = "End date range for quota query. Use -MM-dd 
as the date format, e.g. startDate=2009-06-03.")
+private Date endDate;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
required = true, description = "Start date range quota query. Use -MM-dd as 
the date format, e.g. startDate=2009-06-01.")
+private Date startDate;
+
+@Parameter(name = ApiConstants.TYPE, type = CommandType.INTEGER, 
description = "List quota usage records for the specified usage type")
+private Integer usageType;
+
+@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, 
entityType = AccountResponse.class, description = "List usage records for the 
specified account")
+private Long accountId;
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public Integer getUsageType() {
+return usageType;
+}
+
+public void setUsageType(Integer usageType) {
+this.usageType = usageType;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public Date getEndDate() {
+return _responseBuilder.startOfNextDay(endDate == null ? new 
Date() : endDate);
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
+}
+
 

[GitHub] cloudstack pull request: Quota

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784658
  
--- Diff: engine/schema/src/com/cloud/usage/UsageVO.java ---
@@ -328,4 +339,48 @@ public Date getStartDate() {
 public Date getEndDate() {
 return endDate;
 }
+
+public void setId(Long id) {
+this.id = id;
+}
+
+public void setType(String type) {
+this.type = type;
+}
+
+public void setStartDate(Date startDate) {
+this.startDate = startDate;
--- End diff --

Not part of quota changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784665
  
--- Diff: framework/db/src/com/cloud/utils/db/Transaction.java ---
@@ -35,18 +35,15 @@
 if (currentTxn != null) {
 databaseId = currentTxn.getDatabaseId();
 }
-TransactionLegacy txn = TransactionLegacy.open(name, databaseId, 
false);
-try {
-//if (txn.dbTxnStarted()){
-//String warnMsg = "Potential Wrong Usage: 
TRANSACTION.EXECUTE IS WRAPPED INSIDE ANOTHER DB TRANSACTION!";
-//s_logger.warn(warnMsg, new 
CloudRuntimeException(warnMsg));
-//}
+try (final TransactionLegacy txn = TransactionLegacy.open(name, 
databaseId, false)) {
+// if (txn.dbTxnStarted()){
+// String warnMsg = "Potential Wrong Usage: 
TRANSACTION.EXECUTE IS WRAPPED INSIDE ANOTHER DB TRANSACTION!";
+// s_logger.warn(warnMsg, new CloudRuntimeException(warnMsg));
+// }
--- End diff --

Not part of quota changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784647
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/usage/GetUsageRecordsCmd.java 
---
@@ -111,6 +111,30 @@ public Long getProjectId() {
 public String getUsageId() {
 return usageId;
 }
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
+}
+
+public void setStartDate(Date startDate) {
+this.startDate = startDate;
--- End diff --

Not part of quota changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784609
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaStatementCmd.java
 ---
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.command;
+
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.QuotaResponseBuilder;
+import org.apache.cloudstack.api.response.QuotaStatementResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.quota.vo.QuotaUsageVO;
+import org.apache.cloudstack.api.response.QuotaStatementItemResponse;
+
+import com.cloud.user.Account;
+
+@APICommand(name = "quotaStatement", responseObject = 
QuotaStatementItemResponse.class, description = "Create a quota statement", 
since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = 
false)
+public class QuotaStatementCmd extends BaseCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(QuotaStatementCmd.class);
+
+private static final String s_name = "quotastatementresponse";
+
+@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, 
required = true, description = "Optional, Account Id for which statement needs 
to be generated")
+private String accountName;
+
+@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, 
required = true, entityType = DomainResponse.class, description = "Optional, If 
domain Id is given and the caller is domain admin then the statement is 
generated for domain.")
+private Long domainId;
+
+@Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, 
required = true, description = "End date range for quota query. Use -MM-dd 
as the date format, e.g. startDate=2009-06-03.")
+private Date endDate;
+
+@Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, 
required = true, description = "Start date range quota query. Use -MM-dd as 
the date format, e.g. startDate=2009-06-01.")
+private Date startDate;
+
+@Parameter(name = ApiConstants.TYPE, type = CommandType.INTEGER, 
description = "List quota usage records for the specified usage type")
+private Integer usageType;
+
+@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, 
entityType = AccountResponse.class, description = "List usage records for the 
specified account")
+private Long accountId;
+
+@Inject
+QuotaResponseBuilder _responseBuilder;
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public Integer getUsageType() {
+return usageType;
+}
+
+public void setUsageType(Integer usageType) {
+this.usageType = usageType;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public Date getEndDate() {
+return _responseBuilder.startOfNextDay(endDate == null ? new 
Date() : endDate);
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
--- End diff 

[GitHub] cloudstack pull request: Quota

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784636
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/usage/GetUsageRecordsCmd.java 
---
@@ -111,6 +111,30 @@ public Long getProjectId() {
 public String getUsageId() {
 return usageId;
 }
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public void setEndDate(Date endDate) {
+this.endDate = endDate;
--- End diff --

Not part of quota changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784827
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaBalanceResponse.java
 ---
@@ -0,0 +1,153 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.response;
+
+import java.math.BigDecimal;
+import java.math.RoundingMode;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+import com.google.gson.annotations.SerializedName;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
+
+import com.cloud.serializer.Param;
+
+public class QuotaBalanceResponse extends BaseResponse {
+
+@SerializedName("accountid")
+@Param(description = "account id")
+private Long accountId;
+
+@SerializedName("account")
+@Param(description = "account name")
+private String accountName;
+
+@SerializedName("domain")
+@Param(description = "domain id")
+private Long domainId;
+
+@SerializedName("startquota")
+@Param(description = "quota started with")
+private BigDecimal startQuota;
+
+@SerializedName("endquota")
+@Param(description = "quota by end of this period")
+private BigDecimal endQuota;
+
+@SerializedName("credits")
+@Param(description = "list of credits made during this period")
+private List credits = null;
+
+@SerializedName("startdate")
+@Param(description = "start date")
+private Date startDate = null;
+
+@SerializedName("enddate")
+@Param(description = "end date")
+private Date endDate = null;
+
+@SerializedName("currency")
+@Param(description = "currency")
+private String currency;
+
+public QuotaBalanceResponse() {
+super();
+credits = new ArrayList();
+}
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public BigDecimal getStartQuota() {
+return startQuota;
+}
+
+public void setStartQuota(BigDecimal startQuota) {
+this.startQuota = startQuota.setScale(2, RoundingMode.HALF_EVEN);
+}
+
+public BigDecimal getEndQuota() {
+return endQuota;
+}
+
+public void setEndQuota(BigDecimal endQuota) {
+this.endQuota = endQuota.setScale(2, RoundingMode.HALF_EVEN);
+}
+
+public List getCredits() {
+return credits;
+}
+
+public void setCredits(List credits) {
+this.credits = credits;
+}
+
+public void addCredits(QuotaBalanceVO credit) {
+QuotaCreditsResponse cr = new QuotaCreditsResponse();
+cr.setCredits(credit.getCreditBalance());
+cr.setUpdatedOn(credit.getUpdatedOn());
+credits.add(0, cr);
+}
+
+public Date getStartDate() {
+return startDate;
+}
+
+public void setStartDate(Date startDate) {
+this.startDate = startDate;
+}
+
+public Date getEndDate() {
+return endDate;
--- End diff --

Since ``Date``s are mutable, a copy of ``endDate`` should be returned to 
avoid downstream side effects.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your 

[GitHub] cloudstack pull request: Quota

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/768#discussion_r46784820
  
--- Diff: 
plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaBalanceResponse.java
 ---
@@ -0,0 +1,153 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements.  See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership.  The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied.  See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.response;
+
+import java.math.BigDecimal;
+import java.math.RoundingMode;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+import com.google.gson.annotations.SerializedName;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
+
+import com.cloud.serializer.Param;
+
+public class QuotaBalanceResponse extends BaseResponse {
+
+@SerializedName("accountid")
+@Param(description = "account id")
+private Long accountId;
+
+@SerializedName("account")
+@Param(description = "account name")
+private String accountName;
+
+@SerializedName("domain")
+@Param(description = "domain id")
+private Long domainId;
+
+@SerializedName("startquota")
+@Param(description = "quota started with")
+private BigDecimal startQuota;
+
+@SerializedName("endquota")
+@Param(description = "quota by end of this period")
+private BigDecimal endQuota;
+
+@SerializedName("credits")
+@Param(description = "list of credits made during this period")
+private List credits = null;
+
+@SerializedName("startdate")
+@Param(description = "start date")
+private Date startDate = null;
+
+@SerializedName("enddate")
+@Param(description = "end date")
+private Date endDate = null;
+
+@SerializedName("currency")
+@Param(description = "currency")
+private String currency;
+
+public QuotaBalanceResponse() {
+super();
+credits = new ArrayList();
+}
+
+public Long getAccountId() {
+return accountId;
+}
+
+public void setAccountId(Long accountId) {
+this.accountId = accountId;
+}
+
+public String getAccountName() {
+return accountName;
+}
+
+public void setAccountName(String accountName) {
+this.accountName = accountName;
+}
+
+public Long getDomainId() {
+return domainId;
+}
+
+public void setDomainId(Long domainId) {
+this.domainId = domainId;
+}
+
+public BigDecimal getStartQuota() {
+return startQuota;
+}
+
+public void setStartQuota(BigDecimal startQuota) {
+this.startQuota = startQuota.setScale(2, RoundingMode.HALF_EVEN);
+}
+
+public BigDecimal getEndQuota() {
+return endQuota;
+}
+
+public void setEndQuota(BigDecimal endQuota) {
+this.endQuota = endQuota.setScale(2, RoundingMode.HALF_EVEN);
+}
+
+public List getCredits() {
+return credits;
+}
+
+public void setCredits(List credits) {
+this.credits = credits;
+}
+
+public void addCredits(QuotaBalanceVO credit) {
+QuotaCreditsResponse cr = new QuotaCreditsResponse();
+cr.setCredits(credit.getCreditBalance());
+cr.setUpdatedOn(credit.getUpdatedOn());
+credits.add(0, cr);
+}
+
+public Date getStartDate() {
+return startDate;
--- End diff --

Since ``Date``s are mutable, a copy of ``startDate`` should be returned to 
avoid downstream side effects.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with