coherence-community / coherence-spring

Coherence Spring Project
https://spring.coherence.community/
Universal Permissive License v1.0
29 stars 23 forks source link

Coherence Spring Data - When updating don't throw NullPointerException #76

Closed ghillert closed 1 year ago

ghillert commented 2 years ago

When doing an update for a non-existing id via CoherenceRepository using e.g.:

this.taskRepository.update(id, myModel -> {
        myModel.setDescription(description);
        return myModel;
    });

then a NullPointerException is thrown:

(Wrapped: Failed request execution for PartitionedCache service on Member(Id=1, Timestamp=2021-10-27 11:06:31.053, Address=10.10.0.15:62339, MachineId=14914, Location=process:61491, Role=IntellijRtJunitJUnitStarter)) java.lang.NullPointerException
    at com.tangosol.util.Base.ensureRuntimeException(Base.java:252)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.service.Grid.tagException(Grid.CDB:61)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.service.grid.partitionedService.PartitionedCache.onInvokeRequest(PartitionedCache.CDB:93)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.service.grid.partitionedService.PartitionedCache$InvokeRequest.run(PartitionedCache.CDB:1)
    at com.tangosol.coherence.component.util.DaemonPool$WrapperTask.run(DaemonPool.CDB:1)
    at com.tangosol.coherence.component.util.DaemonPool$WrapperTask.run(DaemonPool.CDB:32)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.Service$DaemonPool$WrapperTask.run(Service.CDB:12)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.service.grid.PartitionedService$DaemonPool$WrapperTask.run(PartitionedService.CDB:1)
    at com.tangosol.coherence.component.util.DaemonPool$Daemon.onNotify(DaemonPool.CDB:66)
    at com.tangosol.coherence.component.util.Daemon.run(Daemon.CDB:54)
    at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.NullPointerException
    at java.base/java.util.Objects.requireNonNull(Objects.java:221)
    at com.oracle.coherence.examples.todo.server.service.SpringDataTaskService$lambda$update$c3e6a45f$1$0:CD693EBC65EA338BA81B91F7C8B386B4.lambda$update$c3e6a45f$1(Unknown Source)
    at com.oracle.coherence.examples.todo.server.service.SpringDataTaskService$lambda$update$c3e6a45f$1$0:CD693EBC65EA338BA81B91F7C8B386B4.apply(Unknown Source)
    at com.oracle.coherence.repository.AbstractRepositoryBase$lambda$updateFunctionProcessor$b845d0ab$1$0:8240A1DA38A2C604E65046FD1F877E5F.lambda$updateFunctionProcessor$b845d0ab$1(Unknown Source)
    at com.oracle.coherence.repository.AbstractRepositoryBase$lambda$updateFunctionProcessor$b845d0ab$1$0:8240A1DA38A2C604E65046FD1F877E5F.process(Unknown Source)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.service.grid.partitionedService.PartitionedCache$Storage.invoke(PartitionedCache.CDB:17)
    at com.tangosol.coherence.component.util.daemon.queueProcessor.service.grid.partitionedService.PartitionedCache.onInvokeRequest(PartitionedCache.CDB:56)
    ... 8 more

We should throw a more business-friendly exception, possibly even considering returning Spring data exceptions:

See: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/dao/package-summary.html

aseovic commented 2 years ago

Yeah, the exception is not the friendliest, because it has occurred during remote lambda invocation, but it is the correct one.

There are no guarantees that the value is not going to be null, so the code should check for that (and typically create a new instance to update):

this.taskRepository.update(id, myModel -> {
        if (myModel == null) {
            myModel = new MyModel(id);
        }
        myModel.setDescription(description);
        return myModel;
    });

Which can also be done using one of the update method overloads, which allow you to specify a value supplier that should be used to create a new value instance when the existing value is null:

this.taskRepository.update(id, MyModel::setDescription, description, MyModel::new);
ghillert commented 1 year ago

@aseovic Just revisiting this a bit more. The relevant code is in AbstractRepositoryBase in Coherence package com.oracle.coherence.repository:

            if (entity == null && factory != null)
                {
                entity = factory.create(entry.getKey());
                }
            updater.update(entity, value);

I would think that if the entity is null and the factory is null, an IllegalStateException should be throw, without calling the updater with a null argument causing the nullpointerexception.

ghillert commented 1 year ago

Added some documentation explain the situation plus work-around.