elide icon indicating copy to clipboard operation
elide copied to clipboard

Lifecycle hook ordering is not deterministic in v7

Open bladedancer opened this issue 2 years ago • 2 comments

I've a number of hooks attached to my entites for the same phase/operation. It allows us to split the business logic cleanly. In v7.0.0pr6 we were seeing the ordering of hook invocations honoring the annotation order.....now it's not.

Based on the limited testing I've done - in EntityBinding fieldTriggers (and classTriggers) are HashSetValuedHashMap.....to my knowledge this doesn't preserve the insertion order.

Expected Behavior

The hooks should guarantee the execution order.

Possible Solution

Could use an ArrayListValuedHashMap - it'll preserve the order.

Steps to Reproduce (for bugs)

It's very hit and miss to reproduce. If needed I imagine I could do it by faking the hashcodes.

Context

The hooks are chaining business logic. The first one populates default values if not set. The later one does validation. If they happen out of order the validation fails.

Your Environment

  • Elide version used: 7.0.0
  • Environment name and version: openjdk 17.0.6 2023-01-17 LTS
  • Operating System and version: Ubuntu 22

bladedancer avatar Dec 20 '23 18:12 bladedancer

Looking at PR6 I'm probably wrong about the assertion it preserved the order - I was just lucky. Though I have confirmed that playing around with the hooks hashcode changes the order of execution.

This is unexpected - I'd expect the order for the hooks in any phase/operation to be the order in which they were defined.

bladedancer avatar Dec 20 '23 20:12 bladedancer

Seems like this would be a relatively simple change to make these deterministic. I'm happy to review a PR if you want to submit a patch.

aklish avatar Dec 29 '23 19:12 aklish

Sorry this one dropped off my radar over christmas. I don't fully follow the oncePerRequest logic but I worked with what was there.

I've created a PR - let me know if you've any questions.

bladedancer avatar Mar 26 '24 10:03 bladedancer