apache · yadvr · Jun 18, 2020 · Jun 5, 2020
diff --git a/usage/pom.xml b/usage/pom.xml
@@ -83,11 +83,6 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <excludes>
-                        <exclude>com/cloud/usage/UsageManagerTest.java</exclude>
-                    </excludes>
-                </configuration>
             </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>

diff --git a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
@@ -37,6 +37,7 @@
 import org.apache.cloudstack.quota.QuotaManager;
 import org.apache.cloudstack.quota.QuotaStatement;
 import org.apache.cloudstack.utils.usage.UsageUtils;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -990,7 +991,7 @@ private void createHelperRecord(UsageEventVO event) {
         } else if (isNetworkOfferingEvent(eventType)) {
             createNetworkOfferingEvent(event);
         } else if (isVPNUserEvent(eventType)) {
-            createVPNUserEvent(event);
+            handleVpnUserEvent(event);
         } else if (isSecurityGroupEvent(eventType)) {
             createSecurityGroupEvent(event);
         } else if (isVmSnapshotEvent(eventType)) {
@@ -1763,39 +1764,92 @@ private void createNetworkOfferingEvent(UsageEventVO event) {
         }
     }
 
-    private void createVPNUserEvent(UsageEventVO event) {
+    /**
+     * Handles VPN user create and remove events:
+     * <ul>
+     *     <li>EventTypes#EVENT_VPN_USER_ADD</li>
+     *     <li>EventTypes#EVENT_VPN_USER_ADD</li>
+     * </ul>
+     * if the event received by this method is neither add nor remove, we ignore it.
+     */
+    protected void handleVpnUserEvent(UsageEventVO event) {
+        long accountId = event.getAccountId();
+        Account account = _accountDao.findByIdIncludingRemoved(accountId);
+        long zoneId = event.getZoneId();
+        long userId = event.getResourceId();
 
-        long zoneId = 0L;
+        switch (event.getType()) {
+            case EventTypes.EVENT_VPN_USER_ADD:
+                createUsageVpnUser(event, account);
+                break;
+            case EventTypes.EVENT_VPN_USER_REMOVE:
+                deleteUsageVpnUser(event, account);
+                break;
+            default:
+                s_logger.debug(String.format("The event [type=%s, zoneId=%s, accountId=%s, userId=%s, resourceName=%s, createDate=%s] is neither of type [%s] nor [%s]",
+                        event.getType(), zoneId, accountId, userId, event.getResourceName(), event.getCreateDate(), EventTypes.EVENT_VPN_USER_ADD, EventTypes.EVENT_VPN_USER_REMOVE));
+        }
+    }
 
+    /**
+     * Find and delete, if exists, usage VPN user entries
+     */
+    protected void deleteUsageVpnUser(UsageEventVO event, Account account) {
+        long accountId = account.getId();
         long userId = event.getResourceId();
+        long zoneId = event.getZoneId();
+        long domainId = account.getDomainId();
 
-        if (EventTypes.EVENT_VPN_USER_ADD.equals(event.getType())) {
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Creating VPN user: " + userId + " for account: " + event.getAccountId());
-            }
-            Account acct = _accountDao.findByIdIncludingRemoved(event.getAccountId());
-            String userName = event.getResourceName();
-            UsageVPNUserVO vpnUser = new UsageVPNUserVO(zoneId, event.getAccountId(), acct.getDomainId(), userId, userName, event.getCreateDate(), null);
+        List<UsageVPNUserVO> usageVpnUsers = findUsageVpnUsers(accountId, zoneId, userId, domainId);
+
+        if (CollectionUtils.isEmpty(usageVpnUsers)) {
+            s_logger.warn(String.format("No usage entry for vpn user [%s] assigned to account [%s] domain [%s] and zone [%s] was found.",
+                    userId, accountId, domainId, zoneId));
+        }
+        if (usageVpnUsers.size() > 1) {
+            s_logger.warn(String.format("More than one usage entry for vpn user [%s] assigned to account [%s] domain [%s] and zone [%s]; marking them all as deleted.", userId,
+                    accountId, domainId, zoneId));
+        }
+        for (UsageVPNUserVO vpnUser : usageVpnUsers) {
+            s_logger.debug(String.format("Deleting vpn user [%s] assigned to account [%s] domain [%s] and zone [%s] that was created at [%s].", vpnUser.getUserId(),
+                    vpnUser.getAccountId(), vpnUser.getDomainId(), vpnUser.getZoneId(), vpnUser.getCreated()));
+            vpnUser.setDeleted(new Date());
+            _usageVPNUserDao.update(vpnUser);
+        }
+    }
+
+    /**
+     * Creates an entry for the Usage VPN User.
+     * If there is already an entry in the database with the same accountId, domainId, userId and zoneId, we do not persist a new entry.
+     */
+    protected void createUsageVpnUser(UsageEventVO event, Account account) {
+        long accountId = account.getId();
+        long userId = event.getResourceId();
+        long zoneId = event.getZoneId();
+        long domainId = account.getDomainId();
+
+        List<UsageVPNUserVO> usageVpnUsers = findUsageVpnUsers(accountId, zoneId, userId, domainId);
+
+        if (usageVpnUsers.size() > 0) {
+            s_logger.debug(String.format("We do not need to create the usage VPN user [%s] assigned to account [%s] because it already exists.", userId, accountId));
+        } else {
+            s_logger.debug(String.format("Creating VPN user user [%s] assigned to account [%s] domain [%s], zone [%s], and created at [%s]", userId, accountId, domainId, zoneId,
+                    event.getCreateDate()));
+            UsageVPNUserVO vpnUser = new UsageVPNUserVO(zoneId, accountId, domainId, userId, event.getResourceName(), event.getCreateDate(), null);
             _usageVPNUserDao.persist(vpnUser);
-        } else if (EventTypes.EVENT_VPN_USER_REMOVE.equals(event.getType())) {
-            SearchCriteria<UsageVPNUserVO> sc = _usageVPNUserDao.createSearchCriteria();
-            sc.addAnd("accountId", SearchCriteria.Op.EQ, event.getAccountId());
-            sc.addAnd("userId", SearchCriteria.Op.EQ, userId);
-            sc.addAnd("deleted", SearchCriteria.Op.NULL);
-            List<UsageVPNUserVO> vuVOs = _usageVPNUserDao.search(sc, null);
-            if (vuVOs.size() > 1) {
-                s_logger.warn("More that one usage entry for vpn user: " + userId + " assigned to account: " + event.getAccountId() + "; marking them all as deleted...");
-            }
-            for (UsageVPNUserVO vuVO : vuVOs) {
-                if (s_logger.isDebugEnabled()) {
-                    s_logger.debug("deleting vpn user: " + vuVO.getUserId());
-                }
-                vuVO.setDeleted(event.getCreateDate()); // there really shouldn't be more than one
-                _usageVPNUserDao.update(vuVO);
-            }
         }
     }
 
+    protected List<UsageVPNUserVO> findUsageVpnUsers(long accountId, long zoneId, long userId, long domainId) {
+        SearchCriteria<UsageVPNUserVO> sc = _usageVPNUserDao.createSearchCriteria();
+        sc.addAnd("zoneId", SearchCriteria.Op.EQ, zoneId);
+        sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
+        sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
+        sc.addAnd("userId", SearchCriteria.Op.EQ, userId);
+        sc.addAnd("deleted", SearchCriteria.Op.NULL);
+        return _usageVPNUserDao.search(sc, null);
+    }
+
     private void createSecurityGroupEvent(UsageEventVO event) {
 
         long zoneId = -1L;

diff --git a/usage/src/test/java/com/cloud/usage/UsageManagerImplTest.java b/usage/src/test/java/com/cloud/usage/UsageManagerImplTest.java
@@ -0,0 +1,167 @@
+// 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 com.cloud.usage;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventVO;
+import com.cloud.usage.dao.UsageVPNUserDao;
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class UsageManagerImplTest {
+
+    @Spy
+    @InjectMocks
+    private UsageManagerImpl usageManagerImpl;
+
+    @Mock
+    private UsageVPNUserDao usageVPNUserDaoMock;
+
+    @Mock
+    private AccountDao accountDaoMock;
+
+    @Mock
+    private UsageVPNUserVO vpnUserMock;
+
+    @Mock
+    private AccountVO accountMock;
+
+    @Mock
+    private UsageEventVO usageEventVOMock;
+
+    private long accountMockId = 1l;
+    private long acountDomainIdMock = 2l;
+
+    @Before
+    public void before() {
+        Mockito.when(accountMock.getId()).thenReturn(accountMockId);
+        Mockito.when(accountMock.getDomainId()).thenReturn(acountDomainIdMock);
+
+        Mockito.doReturn(accountMock).when(accountDaoMock).findByIdIncludingRemoved(Mockito.anyLong());
+
+    }
+
+    @Test
+    public void createUsageVpnUserTestUserExits() {
+        List<UsageVPNUserVO> vpnUsersMock = new ArrayList<UsageVPNUserVO>();
+        vpnUsersMock.add(vpnUserMock);
+
+        Mockito.doReturn(vpnUsersMock).when(usageManagerImpl).findUsageVpnUsers(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong());
+
+        usageManagerImpl.createUsageVpnUser(usageEventVOMock, accountMock);
+
+        Mockito.verify(usageVPNUserDaoMock, Mockito.never()).persist(Mockito.any(UsageVPNUserVO.class));
+
+    }
+
+    @Test
+    public void createUsageVpnUserTestUserDoesNotExits() {
+        List<UsageVPNUserVO> vpnUsersMock = new ArrayList<UsageVPNUserVO>();
+
+        Mockito.doReturn(vpnUsersMock).when(usageManagerImpl).findUsageVpnUsers(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doReturn(vpnUserMock).when(usageVPNUserDaoMock).persist(Mockito.any(UsageVPNUserVO.class));
+
+        usageManagerImpl.createUsageVpnUser(usageEventVOMock, accountMock);
+
+        Mockito.verify(usageVPNUserDaoMock, Mockito.times(1)).persist(Mockito.any(UsageVPNUserVO.class));
+
+    }
+
+    @Test
+    public void deleteUsageVpnUserNoUserFound() {
+        List<UsageVPNUserVO> vpnUsersMock = new ArrayList<UsageVPNUserVO>();
+
+        Mockito.doReturn(vpnUsersMock).when(usageManagerImpl).findUsageVpnUsers(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong());
+
+        usageManagerImpl.deleteUsageVpnUser(usageEventVOMock, accountMock);
+
+        Mockito.verify(usageVPNUserDaoMock, Mockito.never()).update(Mockito.any(UsageVPNUserVO.class));
+    }
+
+    @Test
+    public void deleteUsageVpnUserOneUserFound() {
+        List<UsageVPNUserVO> vpnUsersMock = new ArrayList<UsageVPNUserVO>();
+        vpnUsersMock.add(vpnUserMock);
+
+        Mockito.doReturn(vpnUsersMock).when(usageManagerImpl).findUsageVpnUsers(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doNothing().when(usageVPNUserDaoMock).update(Mockito.any(UsageVPNUserVO.class));
+
+        usageManagerImpl.deleteUsageVpnUser(usageEventVOMock, accountMock);
+
+        Mockito.verify(usageVPNUserDaoMock, Mockito.times(1)).update(Mockito.any(UsageVPNUserVO.class));
+    }
+
+    @Test
+    public void deleteUsageVpnUserMultipleUsersFound() {
+        List<UsageVPNUserVO> vpnUsersMock = new ArrayList<UsageVPNUserVO>();
+        vpnUsersMock.add(vpnUserMock);
+        vpnUsersMock.add(Mockito.mock(UsageVPNUserVO.class));
+        vpnUsersMock.add(Mockito.mock(UsageVPNUserVO.class));
+
+        Mockito.doReturn(vpnUsersMock).when(usageManagerImpl).findUsageVpnUsers(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doNothing().when(usageVPNUserDaoMock).update(Mockito.any(UsageVPNUserVO.class));
+
+        usageManagerImpl.deleteUsageVpnUser(usageEventVOMock, accountMock);
+
+        Mockito.verify(usageVPNUserDaoMock, Mockito.times(3)).update(Mockito.any(UsageVPNUserVO.class));
+    }
+
+    @Test
+    public void handleVpnUserEventTestAddUser() {
+        Mockito.when(this.usageEventVOMock.getType()).thenReturn(EventTypes.EVENT_VPN_USER_ADD);
+        Mockito.doNothing().when(this.usageManagerImpl).createUsageVpnUser(usageEventVOMock, accountMock);
+
+        this.usageManagerImpl.handleVpnUserEvent(usageEventVOMock);
+
+        Mockito.verify(usageManagerImpl).createUsageVpnUser(usageEventVOMock, accountMock);
+        Mockito.verify(usageManagerImpl, Mockito.never()).deleteUsageVpnUser(usageEventVOMock, accountMock);
+    }
+
+    @Test
+    public void handleVpnUserEventTestRemoveUser() {
+        Mockito.when(this.usageEventVOMock.getType()).thenReturn(EventTypes.EVENT_VPN_USER_REMOVE);
+        Mockito.doNothing().when(this.usageManagerImpl).deleteUsageVpnUser(usageEventVOMock, accountMock);
+
+        this.usageManagerImpl.handleVpnUserEvent(usageEventVOMock);
+
+        Mockito.verify(usageManagerImpl, Mockito.never()).createUsageVpnUser(usageEventVOMock, accountMock);
+        Mockito.verify(usageManagerImpl).deleteUsageVpnUser(usageEventVOMock, accountMock);
+    }
+
+    @Test
+    public void handleVpnUserEventTestEventIsNeitherAddNorRemove() {
+        Mockito.when(this.usageEventVOMock.getType()).thenReturn("VPN.USER.UPDATE");
+
+        this.usageManagerImpl.handleVpnUserEvent(usageEventVOMock);
+
+        Mockito.verify(usageManagerImpl, Mockito.never()).createUsageVpnUser(usageEventVOMock,accountMock);
+        Mockito.verify(usageManagerImpl, Mockito.never()).deleteUsageVpnUser(usageEventVOMock, accountMock);
+    }
+}