yahoo / elide

Elide is a Java library that lets you stand up a GraphQL/JSON-API web service with minimal effort.
https://elide.io
Other
991 stars 227 forks source link

Update Hook entity per Transaction phase not working as expected #3251

Closed white3km closed 3 weeks ago

white3km commented 3 weeks ago

Previously my project used a hook for updates to pull the original entity from the database and perform some business logic with the database version of the entity. With the update to Elide 7 I've noticed the database version of the entity already has updated data despite being the Pre-Security or Pre-Flush phase.

Expected Behavior

Config

elide:
  defaultPageSize: 10
  maxPageSize: 500
  verboseErrors: true
  json-api:
    enabled: true
    path: ${API_PREFIX:/api}
  graphql:
    enabled: false
  api-docs:
    path: /doc
    enabled: true
    version: openapi_3_0
  async:
    enabled: false
  aggregation-store:
    enabled: false

Entity

@Slf4j
@Entity
@Setter
@LifeCycleHookBinding(
    operation = LifeCycleHookBinding.Operation.UPDATE,
    phase = LifeCycleHookBinding.TransactionPhase.PRESECURITY,
    hook = AlphaHook.class)
@LifeCycleHookBinding(
    operation = LifeCycleHookBinding.Operation.UPDATE,
    phase = LifeCycleHookBinding.TransactionPhase.PREFLUSH,
    hook = AlphaHook.class)
@LifeCycleHookBinding(
    operation = LifeCycleHookBinding.Operation.UPDATE,
    phase = LifeCycleHookBinding.TransactionPhase.PRECOMMIT,
    hook = AlphaHook.class)
@Table(name = "alpha")
@Include(name = "alpha")
public class Alpha {
  private UUID id;
  private String textValue;

 @Id
  @GeneratedValue
  @Column(
      name = "id",
      columnDefinition = "uuid",
      unique = true,
      nullable = false,
      updatable = false)
  public UUID getId() {
    return id;
  }

  @Column(name = "text_value", nullable = true)
  public String getTextValue() {
    return textValue;
  }

Repository

public interface AlphaRespository extends JpaRepository<Alpha, UUID> {}

Hook

@Slf4j
public class AlphaHook implements LifeCycleHook<Alpha> {

  private final AlphaRespository alphaRespository;

  public AlphaHook(AlphaRespository alphaRespository) {
    this.alphaRespository = alphaRespository;
  }

  @Override
  public void execute(
      LifeCycleHookBinding.Operation operation,
      LifeCycleHookBinding.TransactionPhase phase,
      Alpha elideEntity,
      RequestScope requestScope,
      Optional<ChangeSpec> changes) {

    changes.ifPresentOrElse(
        (cs) ->
            log.info(
                "PHASE = "
                    + phase
                    + ", CHANGE SPEC: "
                    + cs.getFieldName()
                    + " - "
                    + cs.getOriginal()
                    + " changed to "
                    + cs.getModified()),
        () -> log.info("PHASE = " + phase + ", NO CHANGE SPEC"));

    Optional<Alpha> alphaOptional = alphaRespository.findById(elideEntity.getId());
    Alpha original = null;
    if (alphaOptional.isPresent()) {
      original = alphaOptional.get();
    }
    log.info(
        "PHASE = "
            + phase
            + ", DATABASE VALUE = "
            + (original != null ? original.getTextValue() : "not found")
            + ", HOOK VALUE = "
            + elideEntity.getTextValue());
  }
}

Patch Request (/api/alpha/0cc11d88-a853-4cf0-9848-e14490b51313)

{
  "data": {
    "id": "0cc11d88-a853-4cf0-9848-e14490b51313",
    "type": "alpha",
    "attributes": {
      "textValue": "banana"
    }
  }
}

Expected Flow

  1. Alpha has database value of 'apple' for text_value
  2. Patch Request is sent with banana for textValue
  3. Hook runs for PRESECURITY (Repository returns entity with text_value = apple)
  4. Hook runs for PREFLUSH (Repository returns entity with text_value = apple)
  5. Object is saved to transaction
  6. Hook runs for PRECOMMIT (Repository returns entity with text_value = banana)
  7. Transaction is committed

Expected Log

PHASE = PRESECURITY, NO CHANGE SPEC
PHASE = PRESECURITY, DATABASE VALUE = apple, HOOK VALUE = banana
PHASE = PREFLUSH, NO CHANGE SPEC
PHASE = PREFLUSH, DATABASE VALUE = apple, HOOK VALUE = banana
PHASE = PRECOMMIT, NO CHANGE SPEC
PHASE = PRECOMMIT, DATABASE VALUE = banana, HOOK VALUE = banana

Current Behavior

Seems like the update is being saved to the transaction earlier than PRE hooks.

Actual Log

PHASE = PRESECURITY, NO CHANGE SPEC
PHASE = PRESECURITY, DATABASE VALUE = banana, HOOK VALUE = banana
PHASE = PREFLUSH, NO CHANGE SPEC
PHASE = PREFLUSH, DATABASE VALUE = banana, HOOK VALUE = banana
PHASE = PRECOMMIT, NO CHANGE SPEC
PHASE = PRECOMMIT, DATABASE VALUE = banana, HOOK VALUE = banana

If I move the hooks to the textValue field

@LifeCycleHookBinding(
          operation = LifeCycleHookBinding.Operation.UPDATE,
          phase = LifeCycleHookBinding.TransactionPhase.PRESECURITY,
          hook = AlphaHook.class)
  @LifeCycleHookBinding(
          operation = LifeCycleHookBinding.Operation.UPDATE,
          phase = LifeCycleHookBinding.TransactionPhase.PREFLUSH,
          hook = AlphaHook.class)
  @LifeCycleHookBinding(
          operation = LifeCycleHookBinding.Operation.UPDATE,
          phase = LifeCycleHookBinding.TransactionPhase.PRECOMMIT,
          hook = AlphaHook.class)
  @Column(name = "text_value", nullable = true)
  public String getTextValue() {
    return textValue;
  }

I still get the same result for database value

PHASE = PRESECURITY, CHANGE SPEC: textValue - apple changed to banana
PHASE = PRESECURITY, DATABASE VALUE = banana, HOOK VALUE = banana
PHASE = PREFLUSH, CHANGE SPEC: textValue - apple changed to banana
PHASE = PREFLUSH, DATABASE VALUE = banana, HOOK VALUE = banana
PHASE = PRECOMMIT, CHANGE SPEC: textValue - apple changed to banana
PHASE = PRECOMMIT, DATABASE VALUE = banana, HOOK VALUE = banana

If I change the operation to Delete

@Slf4j
@Entity
@Setter
@LifeCycleHookBinding(
    operation = LifeCycleHookBinding.Operation.DELETE,
    phase = LifeCycleHookBinding.TransactionPhase.PRESECURITY,
    hook = AlphaHook.class)
@LifeCycleHookBinding(
    operation = LifeCycleHookBinding.Operation.DELETE,
    phase = LifeCycleHookBinding.TransactionPhase.PREFLUSH,
    hook = AlphaHook.class)
@LifeCycleHookBinding(
    operation = LifeCycleHookBinding.Operation.DELETE,
    phase = LifeCycleHookBinding.TransactionPhase.PRECOMMIT,
    hook = AlphaHook.class)
@Table(name = "alpha")
@Include(name = "alpha"
public class Alpha { ... }

Then the phases seem to be performing properly (entity is not found after flush)

PHASE = PRESECURITY, NO CHANGE SPEC
PHASE = PRESECURITY, DATABASE VALUE = apple, HOOK VALUE = apple
PHASE = PREFLUSH, NO CHANGE SPEC
PHASE = PREFLUSH, DATABASE VALUE = apple, HOOK VALUE = apple
PHASE = PRECOMMIT, NO CHANGE SPEC
PHASE = PRECOMMIT, DATABASE VALUE = not found, HOOK VALUE = apple

Context

While I realize you can annotate individual fields and utilize the ChangeSpec for individual field changes, we found that we needed the full entity to run our business logic on update operations. That's why we prefer the class level hook and with repository approach.

Your Environment

justin-tay commented 3 weeks ago

Elide 7 adds support in the Spring autoconfiguration to use the Spring Platform Transaction Manager by default. This means that your Repository operations are taking place in the same transaction as what Elide is using. This means that the persistent entity being passed into the hook is actually the exact same persistent entity instance as that retrieved using findById since it's essentially sharing the same EntityManager. ie. elideEntity == original

Prior to Elide 7, the Elide operations and the Repository operations would be performed in separate transactions since they are not aware of each other. So in that case your Repository operation will be pulling from the database as it's creating a separate transaction and the entity isn't already present in the EntityManager associated with that transaction.

To achieve your previous behavior of creating a new transaction to retrieve the old records you can inject a @Service with @Transactional(propagation = Propagation.REQUIRES_NEW) into your AlphaHook so that Spring will start a new transaction when you call findById. Your hook will then use the AlphaService instead of directly using the AlphaRepository.

@Service
@Transactional(propagation = Propagation.REQUIRES_NEW)
public class AlphaService {
    private final AlphaRepository alphaRepository;

    public AlphaService (AlphaRepository alphaRepository) {
        this.alphaRepository = alphaRepository;
    }

    public Optional<Alpha> findById(UUID id) {
        return alphaRepository.findById(id);
    }
}
white3km commented 3 weeks ago

Interesting, that's very helpful information. I see the default in the config now (https://github.com/yahoo/elide/blob/master/elide-spring/elide-spring-boot-autoconfigure/src/main/java/com/yahoo/elide/spring/orm/jpa/PlatformJpaTransaction.java).

Thanks for the info and the quick response!