Updates or creates an entity based on if a value is present in an Optional

Question

I need to get car info from a 3rd party web service and persist the data in my application DB. If my DB already has the car, I only update property values that may have changed. Otherwise, I create and save the car:

@Transactional //Spring Transaction
void saveCars() {

   //get CarInfo from a webservice
   List<CarInfo> carInfos = getCarInfo();

   // now get each Car from my application DB
   // that correspond to each CarInfo.
   Set<Car> cars = mySpringCarRepo.findByCarInfoIdIn(
       carInfos.stream().map(c -> c.getId())
           .collect(Collectors.toSet());

   for (CarInfo carInfo : carInfos) {

      // If a car for the carInfo 
      // is already in my application
      // just update it if some carInfo has changed.
      // If a car is not there, create and save it to DB.
      Car car = cars
                 .stream()
                 .filter(c -> c.getCarInfoId().equals(carInfo.getId())
                 .findAny()
                 .map(c -> updateCar(carInfo, car))
                 .orElseGet(() -> createCar(carInfo);

       mySpringCarRepo.save(car);
   }
}

I have the following questions / concerns about this code:

1. Is this an inappropriate use of Optional's map method? I'm not mapping to a new object but rather just returning the same car with some properties potentially updated.
2. The save method is really only required to be called for cars created in the orElseGet. Due to being in a transaction, the updates to the car will be persisted without the save call. It seems redundant and perhaps confusing to call save.

Effective Java 3rd edition states that many uses of isPresent can profitably be replaced by other Optional methods. So that has led me to the path of using map & orElseGet and the fact that Java 8 does not have a ifPresentOrElse method on Optional.

Answer 1

Regarding your questions

Is this an inappropriate use of Optional's map method? I'm not mapping to a new object but rather just returning the same car with some properties potentially updated.

I think it's appropriate because it might return an updated object. Unfortunately (as you said) Java 8 doesn't have ifPresentOrElse.

The save method is really only required to be called for cars created in the orElseGet.

You can change the orElseGet with orElseGet(() -> mySpringCarRepo.save(createCar(carInfo))) and remove the final save to the db.

Performances

The complexity of the method is O(c*ci) where c is the number of cars in your DB and ci is the number of CarInfo. If you store the Cars in a Map instead of a Set you can improve it to O(ci).

void saveCars() {
    // get CarInfo from a webservice
    List<CarInfo> carInfos = getCarInfo();
    // Get cars from DB that correspond to each CarInfo
    Map<Long,Car> cars = mySpringCarRepo
            .findByCarInfoIdIn( getCarInfoIds(carInfos) )
            .stream()
            .collect( Collectors.toMap(Car::getCarInfoId,Function.identity()));
    
    for (CarInfo carInfo : carInfos) {
        // Returns Optional<Car>
        Optional.ofNullable(cars.get(carInfo.getId()))
        // Update car if already exists
        .map(c -> updateCar(carInfo, c))
        // Save car otherwise
        .orElseGet(() -> mySpringCarRepo.save(createCar(carInfo)));
    }
}

Or if you don't mind to query the db for each CarInfo, than the method is shorter:

void saveCars() {
    getCarInfo().stream()
        .forEach(carInfo -> 
                    // returns Optional<Car>
                    mySpringCarRepo.findByCarInfoId(carInfo.getId()) 
                    // Update car if already exists
                    .map(car -> updateCar(carInfo, car))
                    // Save car otherwise
                    .orElseGet(() -> mySpringCarRepo.save(createCar(carInfo))));
}

Note 1: I haven't fully tested the code, it's just to give you an idea how to improve it.

Note 2: the performance gain might be irrelevant due to the DB and network latency, but with the second approach you should save some memory space.

Naming

The method name saveCars is not very appropriate, a better name might be updateCars or syncCars.