Skip to content

Commit

Permalink
Disable "open session in view".
Browse files Browse the repository at this point in the history
This commit makes the `milages` association lazy (the default) and also disables the open session in view interceptor.

By storing the last recorded milage on the bike entity itself, the need for eagerly accessing the collection goes away.

The commit also removes `@Transactional` from the bikes controller while introducing a dedicated service for manipulating bikes.
The service takes care of the transaction handling from there on and reads the milages in a controlled way when manipulating them.

One fresh wound is the test of the biker controller. It loads the service into the test slice now and passes the mocked repository.
This shall be changed at some later point.
  • Loading branch information
michael-simons committed Nov 2, 2019
1 parent 2c7cb85 commit 9c670d7
Show file tree
Hide file tree
Showing 9 changed files with 185 additions and 111 deletions.
52 changes: 17 additions & 35 deletions src/main/java/ac/simons/biking2/bikes/BikeEntity.java
Expand Up @@ -16,6 +16,7 @@
package ac.simons.biking2.bikes;

import java.io.Serializable;
import java.math.BigDecimal;
import java.time.LocalDate;
import java.time.OffsetDateTime;
import java.util.ArrayList;
Expand All @@ -26,7 +27,6 @@
import javax.persistence.Embeddable;
import javax.persistence.Embedded;
import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
Expand Down Expand Up @@ -113,11 +113,16 @@ public Link(final String url, final String label) {
@Getter
private LocalDate decommissionedOn;

@OneToMany(fetch = FetchType.EAGER, cascade = CascadeType.ALL, mappedBy = "bike")
@OneToMany(cascade = CascadeType.ALL, mappedBy = "bike")
@OrderBy("recordedOn asc")
@JsonIgnore
private final List<MilageEntity> milages = new ArrayList<>();

@Column(name = "last_milage", nullable = false, precision = 8, scale = 2)
@NotNull
@JsonIgnore
private BigDecimal lastMilage = BigDecimal.ZERO;

@Column(name = "created_at", nullable = false)
@NotNull
@JsonIgnore
Expand Down Expand Up @@ -151,49 +156,26 @@ public boolean decommission(final LocalDate decommissionDate) {
public synchronized MilageEntity addMilage(final LocalDate recordedOn, final double amount) {

if (!this.milages.isEmpty()) {
final MilageEntity lastMilage = this.milages.get(this.milages.size() - 1);
LocalDate nextValidDate = lastMilage.getRecordedOn().plusMonths(1);
var lastRecordedMilage = this.milages.get(this.milages.size() - 1);
var nextValidDate = lastRecordedMilage.getRecordedOn().plusMonths(1);
if (!recordedOn.equals(nextValidDate)) {
throw new IllegalArgumentException("Next valid date for milage is " + nextValidDate);
}
if (lastMilage.getAmount().doubleValue() > amount) {
throw new IllegalArgumentException("New amount must be greater than or equal " + lastMilage.getAmount().toPlainString());
if (lastRecordedMilage.getAmount().doubleValue() > amount) {
throw new IllegalArgumentException("New amount must be greater than or equal " + lastRecordedMilage.getAmount().toPlainString());
}
}
final MilageEntity milage = new MilageEntity(this, recordedOn.withDayOfMonth(1), amount);
this.milages.add(milage);
return milage;
var newRecordedMilage = new MilageEntity(this, recordedOn.withDayOfMonth(1), amount);
this.milages.add(newRecordedMilage);
this.lastMilage = newRecordedMilage.getAmount();
return newRecordedMilage;
}

/**
* @return The total milage that has been recorded here.
*/
@JsonProperty
public int getMilage() {

if (this.milages.isEmpty()) {
return 0;
}

return this.getLastMilage() - this.getFirstMilage();
}

/**
* @return The first milage recorded with this app.
*/
int getFirstMilage() {
return this.milages.isEmpty() ? 0 : this.milages.get(0).getAmount().intValue();
}

/**
* @return The last milage recorded with this app.
* @return The last milage recorded for this bike.
*/
@JsonProperty
public int getLastMilage() {
return this.milages.isEmpty() ? 0 : this.milages.get(this.milages.size() - 1).getAmount().intValue();
}

public boolean hasMilages() {
return !this.milages.isEmpty();
return this.lastMilage.intValue();
}
}
108 changes: 108 additions & 0 deletions src/main/java/ac/simons/biking2/bikes/BikeService.java
@@ -0,0 +1,108 @@
/*
* Copyright 2019 michael-simons.eu.
*
* 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 ac.simons.biking2.bikes;

import java.util.List;
import java.util.Optional;

import org.springframework.data.domain.Sort;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;

/**
* @author Michael J. Simons
* @since 2019-11-02
*/
@Service
@RequiredArgsConstructor(access = AccessLevel.PACKAGE)
class BikeService {

private final BikeRepository bikeRepository;

@Transactional
public BikeEntity createBike(final BikeCmd newBike) {

final BikeEntity bike = new BikeEntity(newBike.getName(), newBike.boughtOnAsLocalDate());
bike.setColor(newBike.getColor());
bike.addMilage(newBike.boughtOnAsLocalDate().withDayOfMonth(1), 0);

return this.bikeRepository.save(bike);
}

@Transactional(readOnly = true)
public List<BikeEntity> getBikes(final boolean all) {

List<BikeEntity> rv;
if (all) {
rv = bikeRepository.findAll(Sort.by("boughtOn", "decommissionedOn", "name").ascending());
} else {
rv = bikeRepository.findByDecommissionedOnIsNull(Sort.by("name").ascending());
}
return rv;
}

@Transactional
public MilageEntity createMilage(final Integer id, final NewMilageCmd cmd) {

final BikeEntity bike = bikeRepository.findById(id).orElseThrow(BikeNotFoundException::new);

MilageEntity rv;
if (bike.getDecommissionedOn() != null) {
throw new BikeAlreadyDecommissionedException();
} else {
rv = bike.addMilage(cmd.recordedOnAsLocalDate(), cmd.getAmount());
this.bikeRepository.save(bike);
}

return rv;
}

@Transactional
public BikeEntity updateBike(final Integer id, final BikeCmd updatedBike) {

final BikeEntity bike = bikeRepository.findById(id).orElseThrow(BikeNotFoundException::new);

if (bike.getDecommissionedOn() != null) {
throw new BikeAlreadyDecommissionedException();
} else {
bike.setColor(updatedBike.getColor());
bike.decommission(updatedBike.decommissionedOnAsLocalDate());
}
return bike;
}

@Transactional
public BikeEntity updateBikeStory(final Integer id, final StoryCmd newStory) {

final BikeEntity bike = bikeRepository.findById(id).orElseThrow(BikeNotFoundException::new);

if (bike.getDecommissionedOn() != null) {
throw new BikeAlreadyDecommissionedException();
} else {
bike.setStory(Optional.ofNullable(newStory).map(c -> new BikeEntity.Link(c.getUrl(), c.getLabel())).orElse(null));
}
return bike;
}

static class BikeNotFoundException extends RuntimeException {
}

static class BikeAlreadyDecommissionedException extends RuntimeException {
}
}
83 changes: 32 additions & 51 deletions src/main/java/ac/simons/biking2/bikes/BikesController.java
Expand Up @@ -15,19 +15,21 @@
*/
package ac.simons.biking2.bikes;

import ac.simons.biking2.bikes.BikeEntity.Link;
import static ac.simons.biking2.bikes.Messages.ALREADY_DECOMMISSIONED;
import static ac.simons.biking2.bikes.BikesController.Messages.ALREADY_DECOMMISSIONED;
import static ac.simons.biking2.shared.Messages.INVALID_ARGUMENTS;
import static org.springframework.web.bind.annotation.RequestMethod.GET;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
import static org.springframework.web.bind.annotation.RequestMethod.PUT;

import java.util.List;
import java.util.Locale;
import java.util.Optional;

import javax.validation.Valid;

import org.springframework.context.MessageSource;
import org.springframework.context.support.MessageSourceAccessor;
import org.springframework.data.domain.Sort;
import org.springframework.http.HttpStatus;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.validation.BindingResult;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
Expand All @@ -36,10 +38,6 @@
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.server.ResponseStatusException;

import static org.springframework.web.bind.annotation.RequestMethod.GET;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
import static org.springframework.web.bind.annotation.RequestMethod.PUT;

/**
* @author Michael J. Simons
* @since 2014-02-19
Expand All @@ -59,97 +57,80 @@ enum Messages {
}
}

private final BikeRepository bikeRepository;
private final BikeService bikeService;

private final MessageSourceAccessor i18n;

BikesController(final BikeRepository bikeRepository, final MessageSource messageSource) {
this.bikeRepository = bikeRepository;
BikesController(final BikeService bikeService, final MessageSource messageSource) {
this.bikeService = bikeService;
this.i18n = new MessageSourceAccessor(messageSource, Locale.ENGLISH);
}

@RequestMapping(value = "/bikes", method = GET)
public List<BikeEntity> getBikes(@RequestParam(required = false, defaultValue = "false") final boolean all) {
List<BikeEntity> rv;
if (all) {
rv = bikeRepository.findAll(Sort.by("boughtOn", "decommissionedOn", "name").ascending());
} else {
rv = bikeRepository.findByDecommissionedOnIsNull(Sort.by("name").ascending());
}
return rv;

return this.bikeService.getBikes(all);
}

@RequestMapping(value = "/bikes/{id:\\d+}/milages", method = POST)
@PreAuthorize("isAuthenticated()")
public MilageEntity createMilage(@PathVariable final Integer id, @RequestBody @Valid final NewMilageCmd cmd, final BindingResult bindingResult) {

if (bindingResult.hasErrors()) {
throw new IllegalArgumentException(i18n.getMessage(INVALID_ARGUMENTS.key));
}

final BikeEntity bike = bikeRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND));

MilageEntity rv;
if (bike.getDecommissionedOn() != null) {
try {
return this.bikeService.createMilage(id, cmd);
} catch (BikeService.BikeNotFoundException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND);
} catch (BikeService.BikeAlreadyDecommissionedException e) {
throw new IllegalArgumentException(i18n.getMessage(ALREADY_DECOMMISSIONED.key));
} else {
rv = bike.addMilage(cmd.recordedOnAsLocalDate(), cmd.getAmount());
this.bikeRepository.save(bike);
}

return rv;
}

@RequestMapping(value = "/bikes", method = POST)
@PreAuthorize("isAuthenticated()")
public BikeEntity createBike(@RequestBody @Valid final BikeCmd newBike, final BindingResult bindingResult) {

if (bindingResult.hasErrors()) {
throw new IllegalArgumentException(i18n.getMessage(INVALID_ARGUMENTS.key));
}

final BikeEntity bike = new BikeEntity(newBike.getName(), newBike.boughtOnAsLocalDate());
bike.setColor(newBike.getColor());
bike.addMilage(newBike.boughtOnAsLocalDate().withDayOfMonth(1), 0);

return this.bikeRepository.save(bike);
return bikeService.createBike(newBike);
}

@RequestMapping(value = "/bikes/{id:\\d+}", method = PUT)
@PreAuthorize("isAuthenticated()")
@Transactional
public BikeEntity updateBike(@PathVariable final Integer id, @RequestBody @Valid final BikeCmd updatedBike, final BindingResult bindingResult) {

if (bindingResult.hasErrors()) {
throw new IllegalArgumentException(i18n.getMessage(INVALID_ARGUMENTS.key));
}

final BikeEntity bike = bikeRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND));

if (bike.getDecommissionedOn() != null) {
try {
return this.bikeService.updateBike(id, updatedBike);
} catch (BikeService.BikeNotFoundException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND);
} catch (BikeService.BikeAlreadyDecommissionedException e) {
throw new IllegalArgumentException(i18n.getMessage(ALREADY_DECOMMISSIONED.key));
} else {
bike.setColor(updatedBike.getColor());
bike.decommission(updatedBike.decommissionedOnAsLocalDate());
}
return bike;
}

@RequestMapping(value = "/bikes/{id:\\d+}/story", method = PUT)
@PreAuthorize("isAuthenticated()")
@Transactional
public BikeEntity updateBikeStory(@PathVariable final Integer id, @RequestBody(required = false) @Valid final StoryCmd newStory, final BindingResult bindingResult) {

if (bindingResult.hasErrors()) {
throw new IllegalArgumentException(i18n.getMessage(INVALID_ARGUMENTS.key));
}

final BikeEntity bike = bikeRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND));

if (bike.getDecommissionedOn() != null) {
try {
return this.bikeService.updateBikeStory(id, newStory);
} catch (BikeService.BikeNotFoundException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND);
} catch (BikeService.BikeAlreadyDecommissionedException e) {
throw new IllegalArgumentException(i18n.getMessage(ALREADY_DECOMMISSIONED.key));
} else {
bike.setStory(Optional.ofNullable(newStory).map(c -> new Link(c.getUrl(), c.getLabel())).orElse(null));
}
return bike;
}
}
8 changes: 7 additions & 1 deletion src/main/java/ac/simons/biking2/bikes/NewMilageCmd.java
Expand Up @@ -22,20 +22,26 @@
import java.time.ZoneId;
import java.time.ZonedDateTime;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Positive;

import org.springframework.format.annotation.DateTimeFormat;
import lombok.Getter;
import lombok.Setter;

/**
* @author Michael J. Simons, 2014-02-19
* @author Michael J. Simons
* @since 2014-02-19
*/
@Getter @Setter
@JsonIgnoreProperties(ignoreUnknown = true)
class NewMilageCmd {

@DateTimeFormat(iso = DATE_TIME)
@NotNull
private ZonedDateTime recordedOn;

@NotNull
@Positive
private Double amount;

public LocalDate recordedOnAsLocalDate() {
Expand Down

0 comments on commit 9c670d7

Please sign in to comment.