From b86966caf93f250cf567f54da998b35fba8a0a53 Mon Sep 17 00:00:00 2001 From: Keith Woodlock Date: Tue, 2 Oct 2012 12:24:36 +0100 Subject: [PATCH] remove usused warnings, make group data immutable. --- .../platform/api/DataTableApiResource.java | 18 +-- .../platform/api/GroupsApiResource.java | 31 +++-- .../platform/api/data/ClientLookup.java | 19 +-- .../mifosng/platform/api/data/GroupData.java | 128 +++++++++--------- .../platform/client/domain/Client.java | 11 +- .../mifosng/platform/group/domain/Group.java | 3 +- .../service/GroupReadPlatformServiceImpl.java | 2 +- ...WritePlatformServiceJpaRepositoryImpl.java | 4 +- .../saving/domain/DepositAccount.java | 1 - ...DepositAccountReadPlatformServiceImpl.java | 22 --- ...WritePlatformServiceJpaRepositoryImpl.java | 21 ++- 11 files changed, 122 insertions(+), 138 deletions(-) diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/api/DataTableApiResource.java b/mifosng-provider/src/main/java/org/mifosng/platform/api/DataTableApiResource.java index e61ead390..b11f64b6f 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/api/DataTableApiResource.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/api/DataTableApiResource.java @@ -121,20 +121,16 @@ public class DataTableApiResource { .retrieveDataTableGenericResultSet(datatable, apptableId, sqlFields, sqlOrder, null); - boolean genericResultSet = ApiParameterHelper.genericResultSet(uriInfo - .getQueryParameters()); - + String json = ""; + final boolean genericResultSet = ApiParameterHelper.genericResultSet(uriInfo.getQueryParameters()); if (genericResultSet) { - boolean prettyPrints = ApiParameterHelper.prettyPrint(uriInfo - .getQueryParameters()); - return this.apiJsonSerializerService - .serializeGenericResultsetDataToJson(prettyPrints, results); + boolean prettyPrints = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters()); + json = this.apiJsonSerializerService.serializeGenericResultsetDataToJson(prettyPrints, results); } else { - - return this.genericDataService - .generateJsonFromGenericResultsetData(results); + json = this.genericDataService.generateJsonFromGenericResultsetData(results); } - + + return json; } @POST diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/api/GroupsApiResource.java b/mifosng-provider/src/main/java/org/mifosng/platform/api/GroupsApiResource.java index 1cc31a1a5..8801479ad 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/api/GroupsApiResource.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/api/GroupsApiResource.java @@ -1,6 +1,5 @@ package org.mifosng.platform.api; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; @@ -20,7 +19,6 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import org.mifosng.platform.api.commands.GroupCommand; -import org.mifosng.platform.api.data.ClientAccountSummaryCollectionData; import org.mifosng.platform.api.data.ClientLookup; import org.mifosng.platform.api.data.EntityIdentifier; import org.mifosng.platform.api.data.GroupAccountSummaryCollectionData; @@ -94,23 +92,26 @@ public class GroupsApiResource { } boolean prettyPrint = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters()); - boolean template = ApiParameterHelper.template(uriInfo.getQueryParameters()); - + GroupData group = this.groupReadPlatformService.retrieveGroup(groupId); + final Collection clientMembers = this.groupReadPlatformService.retrieveClientMembers(groupId); + Collection availableClients = null; + Collection allowedOffices = null; + + group = new GroupData(group, clientMembers, availableClients, allowedOffices); + + boolean template = ApiParameterHelper.template(uriInfo.getQueryParameters()); + if (template) { + responseParameters.add("allowedClients"); + responseParameters.add("allowedOffices"); - Collection clientMembers = this.groupReadPlatformService.retrieveClientMembers(groupId); - group.setClientMembers(clientMembers); + availableClients = this.clientReadPlatformService.retrieveAllIndividualClientsForLookup(); + availableClients.removeAll(group.clientMembers()); - if (template) { - Collection availableClients = this.clientReadPlatformService.retrieveAllIndividualClientsForLookup(); - availableClients.removeAll(group.getClientMembers()); - group.setAllowedClients(availableClients); + allowedOffices = officeReadPlatformService.retrieveAllOfficesForLookup(); - responseParameters.add("allowedClients"); - - group.setAllowedOffices(new ArrayList(officeReadPlatformService.retrieveAllOfficesForLookup())); - responseParameters.add("allowedOffices"); - } + group = new GroupData(group, group.clientMembers(), availableClients, allowedOffices); + } return this.apiJsonSerializerService.serializeGroupDataToJson(prettyPrint, responseParameters, group); } diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/api/data/ClientLookup.java b/mifosng-provider/src/main/java/org/mifosng/platform/api/data/ClientLookup.java index fdbbe3980..75a186a8b 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/api/data/ClientLookup.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/api/data/ClientLookup.java @@ -5,11 +5,14 @@ import org.apache.commons.lang.StringUtils; public class ClientLookup { private final Long id; + @SuppressWarnings("unused") private final String displayName; - private final Long officeId; - private final String officeName; + @SuppressWarnings("unused") + private final Long officeId; + @SuppressWarnings("unused") + private final String officeName; - public ClientLookup(final Long id, final String firstname, final String lastname, final Long officeId, String officeName) { + public ClientLookup(final Long id, final String firstname, final String lastname, final Long officeId, final String officeName) { this.id = id; StringBuilder displayNameBuilder = new StringBuilder(firstname); @@ -39,18 +42,10 @@ public class ClientLookup { this.officeName = null; } - public Long getId() { - return id; - } - - public String getDisplayName() { - return displayName; - } - @Override public boolean equals(Object obj) { ClientLookup clientLookup = (ClientLookup) obj; - return this.id.equals(clientLookup.getId()); + return this.id.equals(clientLookup.id); } @Override diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/api/data/GroupData.java b/mifosng-provider/src/main/java/org/mifosng/platform/api/data/GroupData.java index 1cfb62b51..3e92ba050 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/api/data/GroupData.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/api/data/GroupData.java @@ -1,46 +1,76 @@ package org.mifosng.platform.api.data; -import java.util.ArrayList; import java.util.Collection; -import java.util.List; +/** + * Immutable data object representing groups. + */ public class GroupData { - - private final Long id; - private final String name; - private final String externalId; - private final Long officeId; - private final String officeName; - private Collection clientMembers = new ArrayList(); - private Collection allowedClients = new ArrayList(); - private Collection allowedOffices = new ArrayList(); + private final Long id; + private final String name; + private final String externalId; + private final Long officeId; + private final String officeName; - public GroupData(Long id, Long officeId, String officeName, String name, String externalId) { - this.id = id; - this.name = name; - this.externalId = externalId; - this.officeId = officeId; - this.officeName = officeName; - } + private final Collection clientMembers; + @SuppressWarnings("unused") + private final Collection allowedClients; + @SuppressWarnings("unused") + private final Collection allowedOffices; - public GroupData(Collection allowedClients, Collection allowedOffices){ - this.id = null; - this.name = null; - this.externalId = null; - this.officeId = null; - this.officeName = null; - this.allowedClients = allowedClients; - this.allowedOffices = allowedOffices; - } + public GroupData( + final Long id, + final Long officeId, + final String officeName, + final String name, + final String externalId) { + this.id = id; + this.officeId = officeId; + this.officeName = officeName; + this.name = name; + this.externalId = externalId; - public Long getId() { - return id; - } + this.clientMembers = null; + this.allowedClients = null; + this.allowedOffices = null; + } - public String getName() { - return name; - } + public GroupData(final GroupData group, final Collection clientMembers, final Collection allowedClients, + final Collection allowedOffices) { + this.id = group.getId(); + this.officeId = group.getOfficeId(); + this.officeName = group.getOfficeName(); + this.name = group.getName(); + this.externalId = group.getExternalId(); + + this.clientMembers = clientMembers; + this.allowedClients = allowedClients; + this.allowedOffices = allowedOffices; + } + + public GroupData(final Collection allowedClients, final Collection allowedOffices) { + this.id = null; + this.officeId = null; + this.officeName = null; + this.name = null; + this.externalId = null; + this.clientMembers = null; + this.allowedClients = allowedClients; + this.allowedOffices = allowedOffices; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public String getExternalId() { + return externalId; + } public Long getOfficeId() { return officeId; @@ -50,31 +80,7 @@ public class GroupData { return officeName; } - public String getExternalId() { - return externalId; - } - - public Collection getClientMembers() { - return clientMembers; - } - - public void setClientMembers(Collection clientMembers) { - this.clientMembers = clientMembers; - } - - public Collection getAllowedClients() { - return allowedClients; - } - - public void setAllowedClients(Collection allowedClients) { - this.allowedClients = allowedClients; - } - - public Collection getAllowedOffices() { - return allowedOffices; - } - - public void setAllowedOffices(Collection allowedOffices) { - this.allowedOffices = allowedOffices; - } -} + public Collection clientMembers() { + return this.clientMembers; + } +} \ No newline at end of file diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/client/domain/Client.java b/mifosng-provider/src/main/java/org/mifosng/platform/client/domain/Client.java index f8bdc282e..0a39f992e 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/client/domain/Client.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/client/domain/Client.java @@ -21,7 +21,6 @@ import org.mifosng.platform.user.domain.AppUser; @Table(name = "m_client") public class Client extends AbstractAuditableCustom { - @SuppressWarnings("unused") @ManyToOne @JoinColumn(name = "office_id", nullable = false) private Office office; @@ -51,7 +50,7 @@ public class Client extends AbstractAuditableCustom { return new Client(clientOffice, firstname, lastname, joiningDate, externalId); } - public Client() { + protected Client() { this.office = null; this.joiningDate = null; this.firstName = null; @@ -141,10 +140,6 @@ public class Client extends AbstractAuditableCustom { this.displayName = this.firstName + " " + this.lastName; } - public Office getOffice() { - return office; - } - /** * Delete is a soft delete. Updates flag on client so it wont appear in query/report results. * @@ -158,4 +153,8 @@ public class Client extends AbstractAuditableCustom { public boolean isDeleted() { return deleted; } + + public boolean isOfficeIdentifiedBy(final Long officeId) { + return this.office.identifiedBy(officeId); + } } \ No newline at end of file diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/group/domain/Group.java b/mifosng-provider/src/main/java/org/mifosng/platform/group/domain/Group.java index a5d390e0e..795b8d970 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/group/domain/Group.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/group/domain/Group.java @@ -22,7 +22,8 @@ import org.mifosng.platform.user.domain.AppUser; @Table(name = "m_group") public class Group extends AbstractAuditableCustom { - @ManyToOne + @SuppressWarnings("unused") + @ManyToOne @JoinColumn(name = "office_id", nullable = false) private Office office; diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupReadPlatformServiceImpl.java b/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupReadPlatformServiceImpl.java index c0113586f..36cc346b4 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupReadPlatformServiceImpl.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupReadPlatformServiceImpl.java @@ -56,7 +56,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { } @Override - public GroupData retrieveGroup(Long groupId) { + public GroupData retrieveGroup(final Long groupId) { try{ this.context.authenticatedUser(); diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupWritePlatformServiceJpaRepositoryImpl.java b/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupWritePlatformServiceJpaRepositoryImpl.java index 5f4a7eaa0..3b45cc198 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupWritePlatformServiceJpaRepositoryImpl.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupWritePlatformServiceJpaRepositoryImpl.java @@ -134,12 +134,12 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla if (command.isClientMembersChanged() && !ObjectUtils.isEmpty(clientMembersArray)) { for (String clientId : clientMembersArray) { - Long id = Long.valueOf(clientId); + final Long id = Long.valueOf(clientId); Client client = this.clientRepository.findOne(id); if (client == null || client.isDeleted()) { throw new ClientNotFoundException(id); } - if (!client.getOffice().getId().equals(command.getOfficeId())){ + if (!client.isOfficeIdentifiedBy(command.getOfficeId())){ String errorMessage = "Group and Client must have the same office."; throw new InvalidOfficeException("client", "attach.to.group", errorMessage); } diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/saving/domain/DepositAccount.java b/mifosng-provider/src/main/java/org/mifosng/platform/saving/domain/DepositAccount.java index ce19ed9de..429c93262 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/saving/domain/DepositAccount.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/saving/domain/DepositAccount.java @@ -98,7 +98,6 @@ public class DepositAccount extends AbstractAuditableCustom { @Column(name = "is_renewal_allowed", nullable = false) private boolean renewalAllowed = false; - @SuppressWarnings("unused") @Column(name = "is_preclosure_allowed", nullable = false) private boolean preClosureAllowed = false; diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountReadPlatformServiceImpl.java b/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountReadPlatformServiceImpl.java index abab527ca..af2cf9f9d 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountReadPlatformServiceImpl.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountReadPlatformServiceImpl.java @@ -5,8 +5,6 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.Collection; -import javax.security.auth.login.AccountException; - import org.joda.time.LocalDate; import org.joda.time.Months; import org.mifosng.platform.api.data.ClientData; @@ -56,26 +54,6 @@ public class DepositAccountReadPlatformServiceImpl implements DepositAccountRead String sql = "select " + mapper.schema() + " where da.is_deleted=0"; return this.jdbcTemplate.query(sql,mapper, new Object[]{}); - - // NOTE: when retrieving a list of deposit accounts, its unlikey we want to much detail so at that point in time, - // not likely that returning what transactions have is useful so removing it. - -// DepositAccountTransactionMapper transactionMapper = new DepositAccountTransactionMapper(); -// String transactionSchema = "Select " + transactionMapper.schema(); -// Collection depositAccountTransactionDatas = this.jdbcTemplate.query(transactionSchema, transactionMapper, new Object[]{}); - -// for (DepositAccountData depositAccountData : depositAccountDatas) { -// List myAccountTransactionDatas = new ArrayList(); -// -// for (DepositAccountTransactionData depositAccountTransactionData : depositAccountTransactionDatas) { -// if (depositAccountData.getId().equals(depositAccountTransactionData.getAccountId())) { -// myAccountTransactionDatas.add(depositAccountTransactionData); -// } -// depositAccountData.setTransactions(myAccountTransactionDatas); -// } -// } - -// return depositAccountDatas; } @Override diff --git a/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java b/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java index 15f346e1d..3d04f556b 100644 --- a/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java +++ b/mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java @@ -363,18 +363,27 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo if (account == null || account.isDeleted()) { throw new DepositAccountNotFoundException(command.getId()); } - if(account.isRenewalAllowed()&&(new LocalDate().isAfter(account.maturesOnDate())||new LocalDate().isEqual(account.maturesOnDate()))){ - if(account.getDepositStatus().equals(DepositAccountStatus.ACTIVE.getValue())){ - final DepositAccount renewedAccount = this.depositAccountAssembler.assembleFrom(account,command); + + // FIXME - KW - extract whats in this if into a method that naturally describes what you are checking. + if (account.isRenewalAllowed() + && (new LocalDate().isAfter(account.maturesOnDate()) || new LocalDate() + .isEqual(account.maturesOnDate()))) { + + // FIXME - KW - rather than getting the status from account and then checking, + // add isActive behaviour on account so all places that use account dont have to repeat this type of checking. + if (account.getDepositStatus().equals( + DepositAccountStatus.ACTIVE.getValue())) { + final DepositAccount renewedAccount = this.depositAccountAssembler + .assembleFrom(account, command); this.depositAccountRepository.save(renewedAccount); account.closeDepositAccount(defaultDepositLifecycleStateMachine()); this.depositAccountRepository.save(account); return new EntityIdentifier(renewedAccount.getId()); - }else{ - throw new DepositAccountReopenException(account.getMaturityDate()); } - }else{ + throw new DepositAccountReopenException(account.getMaturityDate()); } + + throw new DepositAccountReopenException(account.getMaturityDate()); } } \ No newline at end of file