Skip to content

Commit

Permalink
Refactoring, fixing some concurrency bugs, adding test cocerage partl…
Browse files Browse the repository at this point in the history
…y related to that.
  • Loading branch information
TatuJLund committed Jun 22, 2024
1 parent 103236d commit 96d3968
Show file tree
Hide file tree
Showing 14 changed files with 443 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,92 +46,107 @@ public synchronized static ProductDataService getInstance() {

@Override
public synchronized List<Product> getAllProducts() {
randomWait(12);
return products.stream().map(p -> new Product(p))
.collect(Collectors.toList());
synchronized (products) {
randomWait(12);
return products.stream().map(p -> new Product(p))
.collect(Collectors.toList());
}
}

@Override
public synchronized List<Category> getAllCategories() {
randomWait(2);
return categories;
public List<Category> getAllCategories() {
synchronized (categories) {
randomWait(2);
return categories;
}
}

@Override
public synchronized Product updateProduct(Product product) {
randomWait(1);
var p = new Product(product);
if (p.getId() < 0) {
// New product
p.setId(nextProductId++);
products.add(p);
logger.info("Saved a new product ({}) {}", p.getId(),
p.getProductName());
return p;
}
for (int i = 0; i < products.size(); i++) {
if (products.get(i).getId() == p.getId()) {
products.set(i, p);
logger.info("Updated the product ({}) {}", p.getId(),
public Product updateProduct(Product product) {
synchronized (products) {
randomWait(1);
var p = new Product(product);
if (p.getId() < 0) {
// New product
p.setId(nextProductId++);
products.add(p);
logger.info("Saved a new product ({}) {}", p.getId(),
p.getProductName());
return p;
}
}
for (int i = 0; i < products.size(); i++) {
if (products.get(i).getId() == p.getId()) {
products.set(i, p);
logger.info("Updated the product ({}) {}", p.getId(),
p.getProductName());
return p;
}
}

throw new IllegalArgumentException(
"No product with id " + p.getId() + " found");
throw new IllegalArgumentException(
"No product with id " + p.getId() + " found");
}
}

@Override
public synchronized Product getProductById(int productId) {
randomWait(1);
for (int i = 0; i < products.size(); i++) {
if (products.get(i).getId() == productId) {
return new Product(products.get(i));
public Product getProductById(int productId) {
synchronized (products) {
randomWait(1);
for (int i = 0; i < products.size(); i++) {
if (products.get(i).getId() == productId) {
return new Product(products.get(i));
}
}
return null;
}
return null;
}

@Override
public synchronized void deleteProduct(int productId) {
randomWait(1);
Product p = getProductById(productId);
if (p == null) {
throw new IllegalArgumentException(
"Product with id " + productId + " not found");
public void deleteProduct(int productId) {
synchronized (products) {
randomWait(1);
Product p = getProductById(productId);
if (p == null) {
throw new IllegalArgumentException(
"Product with id " + productId + " not found");
}
products.remove(p);
}
products.remove(p);
}

@Override
public Category updateCategory(Category category) {
randomWait(1);
var newCategory = new Category(category);
if (newCategory.getId() < 0) {
newCategory.setId(nextCategoryId++);
categories.add(newCategory);
logger.info("Category {} created", newCategory.getId());
} else {
deleteCategoryInternal(category.getId());
categories.add(newCategory);
logger.info("Category {} updated", newCategory.getId());
synchronized (categories) {
randomWait(1);
var newCategory = new Category(category);
if (newCategory.getId() < 0) {
newCategory.setId(nextCategoryId++);
categories.add(newCategory);
logger.info("Category {} created", newCategory.getId());
} else {
deleteCategoryInternal(category.getId());
categories.add(newCategory);
logger.info("Category {} updated", newCategory.getId());
}
return newCategory;
}
return newCategory;
}

@Override
public void deleteCategory(int categoryId) {
randomWait(1);
if (!getAllProducts().stream().anyMatch(c -> c.getId() == categoryId)) {
throw new IllegalArgumentException(
"Category with id " + categoryId + " not found");
synchronized (categories) {
randomWait(1);
if (!getAllProducts().stream()
.anyMatch(c -> c.getId() == categoryId)) {
throw new IllegalArgumentException(
"Category with id " + categoryId + " not found");
}
deleteCategoryInternal(categoryId);
}
deleteCategoryInternal(categoryId);
logger.info("Category {} deleted", categoryId);
}

public void deleteCategoryInternal(int categoryId) {
private void deleteCategoryInternal(int categoryId) {
if (categories.removeIf(category -> category.getId() == categoryId)) {
getAllProducts().forEach(product -> {
product.getCategory()
Expand All @@ -142,9 +157,11 @@ public void deleteCategoryInternal(int categoryId) {

@Override
public Set<Category> findCategoriesByIds(Set<Integer> categoryIds) {
return categories.stream()
.filter(cat -> categoryIds.contains(cat.getId()))
.collect(Collectors.toSet());
synchronized (categories) {
return categories.stream()
.filter(cat -> categoryIds.contains(cat.getId()))
.collect(Collectors.toSet());
}
}

@Override
Expand All @@ -161,7 +178,7 @@ public void restore(Collection<Product> data) {
}

private void randomWait(int count) {
int wait = 50 + random.nextInt(150);
int wait = 50 + random.nextInt(100);
try {
Thread.sleep(wait * count);
} catch (InterruptedException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.vaadin.tatu.vaadincreate.util.CookieUtil;
import org.vaadin.tatu.vaadincreate.util.Utils;

import com.vaadin.annotations.PreserveOnRefresh;
import com.vaadin.annotations.Push;
import com.vaadin.annotations.StyleSheet;
import com.vaadin.annotations.Theme;
Expand All @@ -42,7 +41,6 @@
@StyleSheet("vaadin://styles/additional-styles.css")
@SuppressWarnings("serial")
@Push(transport = Transport.WEBSOCKET_XHR)
@PreserveOnRefresh
@Viewport("width=device-width, initial-scale=1, maximum-scale=1.0, user-scalable=no")
public class VaadinCreateUI extends UI implements EventBusListener, HasI18N {

Expand All @@ -63,15 +61,15 @@ public class VaadinCreateUI extends UI implements EventBusListener, HasI18N {
protected void init(VaadinRequest request) {
getPage().setTitle("Vaadin Create 23'");
if (!getAccessControl().isUserSignedIn()) {
showLoginView(request);
showLoginView();
} else {
target = getInitialTarget();
showAppLayout();
}
eventBus.registerEventBusListener(this);
}

private void showLoginView(VaadinRequest request) {
private void showLoginView() {
setContent(new LoginView(getAccessControl(), e -> onLogin()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@
import org.vaadin.tatu.vaadincreate.backend.data.Availability;
import org.vaadin.tatu.vaadincreate.backend.data.Category;
import org.vaadin.tatu.vaadincreate.backend.data.Product;
import org.vaadin.tatu.vaadincreate.eventbus.EventBus;
import org.vaadin.tatu.vaadincreate.eventbus.EventBus.EventBusListener;
import org.vaadin.tatu.vaadincreate.i18n.HasI18N;
import org.vaadin.tatu.vaadincreate.locking.LockedObjects;
import org.vaadin.tatu.vaadincreate.locking.LockedObjects.LockingEvent;
import org.vaadin.tatu.vaadincreate.util.Utils;

import com.vaadin.data.ValueContext;
import com.vaadin.data.provider.ListDataProvider;
import com.vaadin.icons.VaadinIcons;
import com.vaadin.shared.Registration;
import com.vaadin.shared.ui.ContentMode;
Expand All @@ -36,7 +32,7 @@
*/
@SuppressWarnings("serial")
public class BookGrid extends Grid<Product>
implements HasI18N, EventBusListener {
implements HasI18N {

private static final String CATEGORIES = "categories";
private static final String IN_STOCK = "in-stock";
Expand All @@ -49,7 +45,6 @@ public class BookGrid extends Grid<Product>
private Registration resizeReg;
private Label availabilityCaption;
private LockedObjects lockedBooks = LockedObjects.get();
private EventBus eventBus = EventBus.get();

private Product editedProduct;
private int edited;
Expand Down Expand Up @@ -112,8 +107,6 @@ public BookGrid() {
// Show all categories the product is in, separated by commas
addColumn(this::formatCategories).setCaption(getTranslation(CATEGORIES))
.setResizable(false).setSortable(false);

eventBus.registerEventBusListener(this);
}

public Product getSelectedRow() {
Expand Down Expand Up @@ -256,7 +249,6 @@ public void detach() {
// It is necessary to remove resize listener upon detach to avoid
// resource leakage.
resizeReg.remove();
eventBus.unregisterEventBusListener(this);
super.detach();
}

Expand All @@ -272,20 +264,5 @@ public void setEdited(Product product) {
editedProduct = product;
}

@SuppressWarnings("unchecked")
@Override
public void eventFired(Object event) {
if (event instanceof LockingEvent && isAttached()) {
var bookEvent = (LockingEvent) event;
getUI().access(() -> {
ListDataProvider<Product> dataProvider = (ListDataProvider<Product>) getDataProvider();
dataProvider.getItems().stream()
.filter(book -> book.getId() == bookEvent.getId())
.findFirst().ifPresent(
product -> dataProvider.refreshItem(product));
});
}
}

private Logger logger = LoggerFactory.getLogger(this.getClass());
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void cancelProduct() {
unlockBook();
}

private void unlockBook() {
public void unlockBook() {
if (editing != null) {
lockedBooks.unlock(Product.class, editing);
editing = null;
Expand Down Expand Up @@ -121,7 +121,8 @@ public void enter(String productId) {
}
}

private Product findProduct(int productId) {
public Product findProduct(int productId) {
logger.info("Fetching product {}", productId);
return service.getProductById(productId);
}

Expand Down
Loading

0 comments on commit 96d3968

Please sign in to comment.