From 1febd39eeedbb337433ff23c09be2c22bc7b5981 Mon Sep 17 00:00:00 2001 From: Vitaly Litvak Date: Wed, 30 Sep 2015 10:22:08 +0300 Subject: [PATCH] For #214 #215 - added possibility to find out allowed maximum number of devices setting for a user, added tests and corresponding UI message --- .../client/controller/SettingsController.java | 42 ++++++++++++------- .../org/traccar/web/client/i18n/Messages.java | 2 + .../traccar/web/client/model/DataService.java | 4 +- .../web/server/model/DataServiceImpl.java | 29 +++++++++---- ...nvalidMaxDeviceNumberForUserException.java | 31 ++++++++++++++ .../org/traccar/web/shared/model/User.java | 26 ++++++++++++ .../web/client/i18n/Messages.properties | 1 + .../web/client/i18n/Messages_ru.properties | 1 + .../web/server/model/DataServiceTest.java | 6 +-- .../model/UserDevicesRestrictionTest.java | 24 +++++++++++ 10 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 src/main/java/org/traccar/web/shared/model/InvalidMaxDeviceNumberForUserException.java diff --git a/src/main/java/org/traccar/web/client/controller/SettingsController.java b/src/main/java/org/traccar/web/client/controller/SettingsController.java index 14b46cee..8ea94c47 100644 --- a/src/main/java/org/traccar/web/client/controller/SettingsController.java +++ b/src/main/java/org/traccar/web/client/controller/SettingsController.java @@ -31,10 +31,7 @@ import org.traccar.web.client.model.NotificationServiceAsync; import org.traccar.web.client.model.UserProperties; import org.traccar.web.client.view.*; -import org.traccar.web.shared.model.ApplicationSettings; -import org.traccar.web.shared.model.NotificationSettings; -import org.traccar.web.shared.model.NotificationTemplate; -import org.traccar.web.shared.model.User; +import org.traccar.web.shared.model.*; import com.google.gwt.core.client.GWT; import com.sencha.gxt.data.shared.ListStore; @@ -87,23 +84,36 @@ public void onSuccess(List result) { @Override public void onAdd() { - new UserDialog( - new User(), - new UserDialog.UserHandler() { + class AddHandler implements UserDialog.UserHandler { + @Override + public void onSave(final User user) { + Application.getDataService().addUser(user, new BaseAsyncCallback(i18n) { @Override - public void onSave(User user) { - Application.getDataService().addUser(user, new BaseAsyncCallback(i18n) { - @Override - public void onSuccess(User result) { - userStore.add(result); - } + public void onSuccess(User result) { + userStore.add(result); + } + @Override + public void onFailure(Throwable caught) { + AlertMessageBox msg; + if (caught instanceof InvalidMaxDeviceNumberForUserException) { + InvalidMaxDeviceNumberForUserException e = (InvalidMaxDeviceNumberForUserException) caught; + msg = new AlertMessageBox(i18n.error(), i18n.errMaxNumOfDevicesExceeded(e.getAllowedDevicesNumber())); + } else { + msg = new AlertMessageBox(i18n.error(), i18n.errUsernameTaken()); + } + msg.addDialogHideHandler(new DialogHideEvent.DialogHideHandler() { @Override - public void onFailure(Throwable caught) { - new AlertMessageBox(i18n.error(), i18n.errUsernameTaken()).show(); + public void onDialogHide(DialogHideEvent event) { + new UserDialog(user, AddHandler.this).show(); } }); + msg.show(); } - }).show(); + }); + } + } + + new UserDialog(new User(), new AddHandler()).show(); } @Override diff --git a/src/main/java/org/traccar/web/client/i18n/Messages.java b/src/main/java/org/traccar/web/client/i18n/Messages.java index 37bafbae..3d1e3d94 100644 --- a/src/main/java/org/traccar/web/client/i18n/Messages.java +++ b/src/main/java/org/traccar/web/client/i18n/Messages.java @@ -406,4 +406,6 @@ String defaultNotificationTemplate(@Select DeviceEventType type, String traceInterval(); String errAccessDenied(); + + String errMaxNumOfDevicesExceeded(int maxNumOfDevices); } diff --git a/src/main/java/org/traccar/web/client/model/DataService.java b/src/main/java/org/traccar/web/client/model/DataService.java index 54b8d27c..cd65690e 100644 --- a/src/main/java/org/traccar/web/client/model/DataService.java +++ b/src/main/java/org/traccar/web/client/model/DataService.java @@ -39,7 +39,7 @@ public interface DataService extends RemoteService { List getUsers(); - User addUser(User user); + User addUser(User user) throws InvalidMaxDeviceNumberForUserException; User updateUser(User user) throws AccessDeniedException; @@ -69,7 +69,7 @@ public interface DataService extends RemoteService { String getTrackerServerLog(short sizeKb); - void saveRoles(List users); + void saveRoles(List users) throws InvalidMaxDeviceNumberForUserException; List getGeoFences(); diff --git a/src/main/java/org/traccar/web/server/model/DataServiceImpl.java b/src/main/java/org/traccar/web/server/model/DataServiceImpl.java index 69474362..ad348dba 100644 --- a/src/main/java/org/traccar/web/server/model/DataServiceImpl.java +++ b/src/main/java/org/traccar/web/server/model/DataServiceImpl.java @@ -211,7 +211,7 @@ public List getUsers() { @RequireUser(roles = { Role.ADMIN, Role.MANAGER }) @RequireWrite @Override - public User addUser(User user) { + public User addUser(User user) throws InvalidMaxDeviceNumberForUserException { User currentUser = getSessionUser(); if (user.getLogin() == null || user.getLogin().isEmpty() || user.getPassword() == null || user.getPassword().isEmpty()) { @@ -227,6 +227,7 @@ public User addUser(User user) { user.setAdmin(false); } user.setManagedBy(currentUser); + validateMaximumNumberOfDevices(user, null, user.getMaxNumOfDevices()); user.setPasswordHashMethod(getApplicationSettings().getDefaultHashImplementation()); user.setPassword(user.getPasswordHashMethod().doHash(user.getPassword(), getApplicationSettings().getSalt())); if (user.getUserSettings() == null) { @@ -241,6 +242,15 @@ public User addUser(User user) { } } + private void validateMaximumNumberOfDevices(User user, Integer originalValue, Integer newValue) throws InvalidMaxDeviceNumberForUserException { + if (user.getManagedBy() != null && newValue != null) { + int allowed = user.getManagedBy().getNumberOfDevicesToDistribute() + (originalValue == null ? 0 : originalValue); + if (allowed - newValue < 0) { + throw new InvalidMaxDeviceNumberForUserException(allowed); + } + } + } + @Transactional @RequireUser @RequireWrite @@ -264,10 +274,8 @@ public User updateUser(User user) throws AccessDeniedException { currentUser.setPasswordHashMethod(getApplicationSettings().getDefaultHashImplementation()); currentUser.setPassword(currentUser.getPasswordHashMethod().doHash(user.getPassword(), getApplicationSettings().getSalt())); } - if(currentUser.getAdmin() || currentUser.getManager()) + if (currentUser.getAdmin() || currentUser.getManager()) { - currentUser.setMaxNumOfDevices(user.getMaxNumOfDevices()); - currentUser.setExpirationDate(user.getExpirationDate()); if (currentUser.getAdmin()) { currentUser.setAdmin(user.getAdmin()); } @@ -781,7 +789,7 @@ public String getTrackerServerLog(short sizeKB) { @RequireUser(roles = { Role.ADMIN, Role.MANAGER }) @RequireWrite @Override - public void saveRoles(List users) { + public void saveRoles(List users) throws InvalidMaxDeviceNumberForUserException { if (users == null || users.isEmpty()) { return; } @@ -794,11 +802,14 @@ public void saveRoles(List users) { user.setAdmin(_user.getAdmin()); } user.setManager(_user.getManager()); - user.setReadOnly(_user.getReadOnly()); user.setArchive(_user.isArchive()); - user.setExpirationDate(_user.getExpirationDate()); - user.setBlocked(_user.isBlocked()); - user.setMaxNumOfDevices(_user.getMaxNumOfDevices()); + if (user.getId() != currentUser.getId()) { + user.setReadOnly(_user.getReadOnly()); + user.setBlocked(_user.isBlocked()); + validateMaximumNumberOfDevices(user, user.getMaxNumOfDevices(), _user.getMaxNumOfDevices()); + user.setMaxNumOfDevices(_user.getMaxNumOfDevices()); + user.setExpirationDate(_user.getExpirationDate()); + } } } diff --git a/src/main/java/org/traccar/web/shared/model/InvalidMaxDeviceNumberForUserException.java b/src/main/java/org/traccar/web/shared/model/InvalidMaxDeviceNumberForUserException.java new file mode 100644 index 00000000..c34b9031 --- /dev/null +++ b/src/main/java/org/traccar/web/shared/model/InvalidMaxDeviceNumberForUserException.java @@ -0,0 +1,31 @@ +/* + * Copyright 2015 Vitaly Litvak (vitavaque@gmail.com) + * + * Licensed 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.traccar.web.shared.model; + +public class InvalidMaxDeviceNumberForUserException extends TraccarException { + private int allowedDevicesNumber; + + public InvalidMaxDeviceNumberForUserException(int allowedDevicesNumber) { + this.allowedDevicesNumber = allowedDevicesNumber; + } + + public InvalidMaxDeviceNumberForUserException() { + } + + public int getAllowedDevicesNumber() { + return allowedDevicesNumber; + } +} diff --git a/src/main/java/org/traccar/web/shared/model/User.java b/src/main/java/org/traccar/web/shared/model/User.java index c2b62d98..6a496eb6 100644 --- a/src/main/java/org/traccar/web/shared/model/User.java +++ b/src/main/java/org/traccar/web/shared/model/User.java @@ -458,6 +458,32 @@ public User getUserWhoReachedLimitOnDevicesNumber() { return null; } + public int getNumberOfDevicesToDistribute() { + Integer maxNumberOfDevices = getMaxNumOfDevices(); + User manager = this; + while (maxNumberOfDevices == null && manager != null) { + maxNumberOfDevices = manager.getMaxNumOfDevices(); + manager = manager.getManagedBy(); + } + if (maxNumberOfDevices == null) { + return Integer.MAX_VALUE; + } + int alreadyDistributedNumberOfDevices = 0; + Set users = manager.getManagedUsers(); + while (!users.isEmpty()) { + Set nextLevelUsers = new HashSet(); + for (User user : users) { + if (user.getMaxNumOfDevices() == null) { + nextLevelUsers.addAll(user.getManagedUsers()); + } else { + alreadyDistributedNumberOfDevices += user.getMaxNumOfDevices(); + } + } + users = nextLevelUsers; + } + return Math.max(0, maxNumberOfDevices - alreadyDistributedNumberOfDevices); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/resources/org/traccar/web/client/i18n/Messages.properties b/src/main/resources/org/traccar/web/client/i18n/Messages.properties index 64344097..1bc112a5 100644 --- a/src/main/resources/org/traccar/web/client/i18n/Messages.properties +++ b/src/main/resources/org/traccar/web/client/i18n/Messages.properties @@ -100,6 +100,7 @@ address = Address # user dialog administrator = Administrator errUsernameTaken = Username is already taken +errMaxNumOfDevicesExceeded = Maximum number of devices should not exceed {0} manager = Manager event = Event deviceEventType = Unknown event diff --git a/src/main/resources/org/traccar/web/client/i18n/Messages_ru.properties b/src/main/resources/org/traccar/web/client/i18n/Messages_ru.properties index 148363a1..aa5dcb19 100644 --- a/src/main/resources/org/traccar/web/client/i18n/Messages_ru.properties +++ b/src/main/resources/org/traccar/web/client/i18n/Messages_ru.properties @@ -100,6 +100,7 @@ address = Адрес # user dialog administrator = Администратор errUsernameTaken = Такое имя пользователя уже занято +errMaxNumOfDevicesExceeded = Максимальное количество устройств не должно быть больше {0} manager = Менеджер event = Событие deviceEventType = Неизвестное событие diff --git a/src/test/java/org/traccar/web/server/model/DataServiceTest.java b/src/test/java/org/traccar/web/server/model/DataServiceTest.java index 2e4fcd25..6d12a929 100644 --- a/src/test/java/org/traccar/web/server/model/DataServiceTest.java +++ b/src/test/java/org/traccar/web/server/model/DataServiceTest.java @@ -134,7 +134,7 @@ public void testDeleteDeviceWithSpecificGeoFence() throws TraccarException { } @Test - public void testDeleteUserWithNotificationSettings() throws AccessDeniedException { + public void testDeleteUserWithNotificationSettings() throws TraccarException { Long originalUserId = injector.getProvider(User.class).get().getId(); User user = new User("test", "test"); @@ -153,7 +153,7 @@ public void testDeleteUserWithNotificationSettings() throws AccessDeniedExceptio } @Test - public void testResetPasswordByAdmin() throws AccessDeniedException { + public void testResetPasswordByAdmin() throws TraccarException { User user = new User("test", "test"); user = dataService.addUser(user); @@ -166,7 +166,7 @@ public void testResetPasswordByAdmin() throws AccessDeniedException { } @Test - public void testResetPasswordByManager() throws AccessDeniedException { + public void testResetPasswordByManager() throws TraccarException { User manager = new User("manager", "manager"); manager.setManager(Boolean.TRUE); manager = dataService.addUser(manager); diff --git a/src/test/java/org/traccar/web/shared/model/UserDevicesRestrictionTest.java b/src/test/java/org/traccar/web/shared/model/UserDevicesRestrictionTest.java index f14604d3..47ddacf5 100644 --- a/src/test/java/org/traccar/web/shared/model/UserDevicesRestrictionTest.java +++ b/src/test/java/org/traccar/web/shared/model/UserDevicesRestrictionTest.java @@ -21,6 +21,7 @@ import org.junit.Test; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; public class UserDevicesRestrictionTest { @@ -89,6 +90,19 @@ public void testStrictHierarchyAddDevicesToLowestLevels() { testStrictHierarchy(0, 0, 0, 0, 0, 0, 0, 0, 0); } + @Test + public void testStrictNumberOfDevicesToDistribute() { + assertEquals(20, m1.getNumberOfDevicesToDistribute()); + assertEquals(20, m2.getNumberOfDevicesToDistribute()); + assertEquals(40, m3.getNumberOfDevicesToDistribute()); + assertEquals(40, m4.getNumberOfDevicesToDistribute()); + assertEquals(10, u1.getNumberOfDevicesToDistribute()); + assertEquals(20, u2.getNumberOfDevicesToDistribute()); + assertEquals(40, u3.getNumberOfDevicesToDistribute()); + assertEquals(10, u4.getNumberOfDevicesToDistribute()); + assertEquals(40, u5.getNumberOfDevicesToDistribute()); + } + private void testStrictHierarchy(int dm1, int dm2, int dm3, @@ -149,6 +163,15 @@ public void createWeirdHierarchy() { m5 = m("m5", 100, m6); } + @Test + public void testWeirdHierarchyNumberOfDevicesToDistribute() { + assertEquals(50, m5.getNumberOfDevicesToDistribute()); + assertEquals(0, m6.getNumberOfDevicesToDistribute()); + assertEquals(0, m7.getNumberOfDevicesToDistribute()); + assertEquals(90, m8.getNumberOfDevicesToDistribute()); + assertEquals(90, m9.getNumberOfDevicesToDistribute()); + } + @Test public void testWeirdHierarchyInitial() { testWeirdHierarchy(100, 50, 50, 50, 50, 50, 50, 50, 50); @@ -198,6 +221,7 @@ private User m(String name, Integer maxNumOfDevices, User... managedUsers) { private User u(String name, Integer maxNumOfDevices) { User user = new User(name); user.setMaxNumOfDevices(maxNumOfDevices); + user.setManagedUsers(Collections.emptySet()); return user; }