Author: bleny Date: 2013-05-15 17:13:20 +0200 (Wed, 15 May 2013) New Revision: 151 Url: http://forge.codelutin.com/projects/franciaflex-magalie/repository/revisions... Log: fix the fact that user is sent on article he cannot access, add docs, use real values for compte location accessibility Modified: trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/Locations.java trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/Location.java trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/MagalieUser.java trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/ArticleStorageService.java trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/BookArticleResult.java trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/RequestedArticleService.java trunk/magalie-services/src/test/java/com/franciaflex/magalie/services/service/ArticleStorageServiceTest.java Modified: trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/Locations.java =================================================================== --- trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/Locations.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/Locations.java 2013-05-15 15:13:20 UTC (rev 151) @@ -33,14 +33,6 @@ public class Locations { - protected static class LocationRequireDriverLicensePredicate implements Predicate<Location> { - - @Override - public boolean apply(Location location) { - return location.isDriverLicenseRequired(); - } - } - protected static class AccessibleLocationPredicate implements Predicate<Location> { protected MagalieUser magalieUser; @@ -51,8 +43,7 @@ @Override public boolean apply(Location location) { - boolean isLocationAccessible = magalieUser.isDriverLicenseOwner() - || ! location.isDriverLicenseRequired(); + boolean isLocationAccessible = magalieUser.getAccreditationLevel() >= location.getRequiredAccreditationLevel(); return isLocationAccessible; } @@ -62,15 +53,7 @@ @Override public int compare(Location location1, Location location2) { - boolean location1RequireDriverLicense = locationRequireDriverLicensePredicate().apply(location1); - boolean location2RequireDriverLicense = locationRequireDriverLicensePredicate().apply(location2); - if (location1RequireDriverLicense && ! location2RequireDriverLicense) { - return -1; - } else if ( ! location1RequireDriverLicense && location2RequireDriverLicense) { - return +1; - } else { - return 0; - } + return Integer.compare(location2.getRequiredAccreditationLevel(), location1.getRequiredAccreditationLevel()); } } @@ -82,10 +65,6 @@ return Predicates.not(accessibleLocationPredicate(magalieUser)); } - public static Predicate<Location> locationRequireDriverLicensePredicate() { - return new LocationRequireDriverLicensePredicate(); - } - public static Comparator<Location> locationRequiringDriverLicenseFirstComparator() { return new LocationRequiringDriverLicenseFirstComparator(); } Modified: trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/Location.java =================================================================== --- trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/Location.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/Location.java 2013-05-15 15:13:20 UTC (rev 151) @@ -80,10 +80,6 @@ return barcode; } - public boolean isDriverLicenseRequired() { - return requiredAccreditationLevel == 9; - } - public boolean isFullLocation() { return fullLocation; } Modified: trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/MagalieUser.java =================================================================== --- trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/MagalieUser.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-persistence/src/main/java/com/franciaflex/magalie/persistence/entity/MagalieUser.java 2013-05-15 15:13:20 UTC (rev 151) @@ -71,7 +71,4 @@ this.accreditationLevel = accreditationLevel; } - public boolean isDriverLicenseOwner() { - return accreditationLevel == 9; - } } Modified: trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/ArticleStorageService.java =================================================================== --- trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/ArticleStorageService.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/ArticleStorageService.java 2013-05-15 15:13:20 UTC (rev 151) @@ -68,6 +68,13 @@ this.serviceContext = serviceContext; } + /** + * Everything starts here, every time an article is requested, a call to + * this method must be used. + * + * @param bookArticleRequest the request gather all the details we must know + * before computing a storage movement order + */ public BookArticleResult bookArticle(BookArticleRequest bookArticleRequest) { Preconditions.checkNotNull(bookArticleRequest.getMagalieUser()); @@ -86,27 +93,32 @@ log.info("will process article request" + bookArticleRequest); } + // we will proceed in three main steps + + // fist, get a snapshot of the actual current state of the stock Iterable<StoredArticle> storedArticles = getStoredArticles(bookArticleRequest.getBuilding(), article); + // second step, sort all those stored article by priority Set<StoredArticle> sortedStoredArticles = sortStoredArticlesByPriority(storedArticles); + // third step, compute actual order with priorities and actual quantities BookArticleResult bookArticleResult = buildStorageMovementOrder(bookArticleRequest, sortedStoredArticles); - StorageMovementOrder storageMovementOrder = bookArticleResult.getStorageMovementOrder(); - MagaliePersistenceContext persistenceContext = serviceContext.getPersistenceContext(); - boolean articleIsAvailable = storageMovementOrder != null; + boolean saveStorageMovementOrder = bookArticleResult.isSuccess(); - if (articleIsAvailable) { + if (saveStorageMovementOrder) { + StorageMovementOrder storageMovementOrder = bookArticleResult.getStorageMovementOrder(); + StorageMovementOrderDao dao = persistenceContext.getStorageMovementOrderDao(); dao.persist(storageMovementOrder); } - updateArticleAvailability(bookArticleRequest, articleIsAvailable); + updateArticleAvailability(bookArticleResult); persistenceContext.commit(); @@ -117,18 +129,22 @@ /** * Add or remove a line in table {@link UnavailableArticle} */ - protected void updateArticleAvailability(BookArticleRequest bookArticleRequest, boolean articleIsAvailable) { + protected void updateArticleAvailability(BookArticleResult bookArticleResult) { - MagaliePersistenceContext persistenceContext = serviceContext.getPersistenceContext(); + BookArticleRequest bookArticleRequest = bookArticleResult.getBookArticleRequest(); - UnavailableArticleDao dao = persistenceContext.getUnavailableArticleDao(); - Article article = bookArticleRequest.getArticle(); Building building = bookArticleRequest.getBuilding(); + MagaliePersistenceContext persistenceContext = serviceContext.getPersistenceContext(); + + UnavailableArticleDao dao = persistenceContext.getUnavailableArticleDao(); + UnavailableArticle unavailableArticle = dao.findByArticle(building, article); + boolean articleIsAvailable = ! bookArticleResult.isArticleUnavailable(); + if (articleIsAvailable) { if (unavailableArticle != null) { @@ -220,8 +236,6 @@ // TODO brendan 15/04/13 refactor - bookArticleResult.setAvailableQuantity(quantity); - if (CollectionUtils.isEmpty(storageMovementOrder.getStorageMovements())) { storageMovementOrder = null; @@ -318,12 +332,16 @@ } + /** + * Get a snapshot of the actual current state of the stock. + */ protected Iterable<StoredArticle> getStoredArticles(Building building, Article article) { MagaliePersistenceContext persistenceContext = serviceContext.getPersistenceContext(); StoredArticleDao storedArticleDao = persistenceContext.getStoredArticleDao(); + // first get all stored articles in the building at known quantities Iterable<StoredArticle> storedArticles = storedArticleDao.findAllForArticleInBuilding(article, building); ImmutableMap<Location, StoredArticle> storedArticlesByLocation = @@ -336,6 +354,8 @@ StorageMovementDao storageMovementDao = persistenceContext.getStorageMovementDao(); + // consider all storage movements, adding quantities when someone added some stock and + // removing quantities if someone has withdraw quantities List<StorageMovement> storageMovements = storageMovementDao.findAllByArticle(article); for (StorageMovement storageMovement : storageMovements) { Modified: trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/BookArticleResult.java =================================================================== --- trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/BookArticleResult.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/BookArticleResult.java 2013-05-15 15:13:20 UTC (rev 151) @@ -27,6 +27,7 @@ import com.franciaflex.magalie.persistence.entity.Location; import com.franciaflex.magalie.persistence.entity.MagalieUser; import com.franciaflex.magalie.persistence.entity.StorageMovementOrder; +import com.google.common.base.Preconditions; import com.google.common.base.Predicate; public class BookArticleResult { @@ -35,8 +36,6 @@ protected BookArticleRequest bookArticleRequest; - protected double availableQuantity; - protected Location inaccessibleLocation; public void setStorageMovementOrder(StorageMovementOrder storageMovementOrder) { @@ -44,6 +43,7 @@ } public StorageMovementOrder getStorageMovementOrder() { + Preconditions.checkState(isSuccess()); return storageMovementOrder; } @@ -81,14 +81,6 @@ return storageMovementOrder == null; } - public void setAvailableQuantity(double availableQuantity) { - this.availableQuantity = availableQuantity; - } - - public double getAvailableQuantity() { - return availableQuantity; - } - public boolean isSuccess() { return ! isArticleInaccessible() && ! isArticleUnavailable(); } Modified: trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/RequestedArticleService.java =================================================================== --- trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/RequestedArticleService.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-services/src/main/java/com/franciaflex/magalie/services/service/RequestedArticleService.java 2013-05-15 15:13:20 UTC (rev 151) @@ -124,10 +124,16 @@ bookArticleResult = articleStorageService.bookArticle(bookArticleRequest); - driverLicenseRequired |= bookArticleResult.isArticleInaccessible() && ! bookArticleResult.isArticleUnavailable(); + boolean articleIsAvailable = ! bookArticleResult.isArticleUnavailable(); - somethingIsAvailable |= ! bookArticleResult.isArticleUnavailable(); + if (articleIsAvailable) { + somethingIsAvailable = true; + + driverLicenseRequired = bookArticleResult.isArticleInaccessible(); + + } + success = bookArticleResult.isSuccess(); } Modified: trunk/magalie-services/src/test/java/com/franciaflex/magalie/services/service/ArticleStorageServiceTest.java =================================================================== --- trunk/magalie-services/src/test/java/com/franciaflex/magalie/services/service/ArticleStorageServiceTest.java 2013-05-13 16:55:20 UTC (rev 150) +++ trunk/magalie-services/src/test/java/com/franciaflex/magalie/services/service/ArticleStorageServiceTest.java 2013-05-15 15:13:20 UTC (rev 151) @@ -29,6 +29,7 @@ import com.franciaflex.magalie.persistence.entity.MagalieUser; import com.franciaflex.magalie.persistence.entity.StorageMovement; import com.franciaflex.magalie.services.AbstractMagalieServiceTest; +import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -101,4 +102,25 @@ Assert.assertEquals(fixedLocation, storageMovements.get(2).getOriginLocation()); } + + @Test + public void testRegression() { + + MagalieUser corinne = fixture("corinne"); + + Article article = fixture("article1"); + + Preconditions.checkState(article.getCode().equals("4034108")); + + BookArticleRequest bookArticleRequest = + new BookArticleRequest(corinne, building, article, 50., destinationLocation, true); + + BookArticleResult bookArticleResult = service.bookArticle(bookArticleRequest); + + Assert.assertFalse(bookArticleResult.isSuccess()); + Assert.assertFalse(bookArticleResult.isArticleUnavailable()); + Assert.assertTrue(bookArticleResult.isArticleInaccessible()); + + } + }