Improves the BaseEntityCache and BaseEntityRef #668
Conversation
`fetch(BaseEntityRef)` delivered the value contained in the provided reference, regardless if a newer version was available in the cache. This would become visible if the entity where the reference was read also comes from an outdated cache. Basically, making this just a commodity method to fetchById. Fixes: SIRI-1039
fetchValue is now deprecated and falls back to fetchCachedValue Fixes: SIRI-1039
fetchValueFromSecondary is now deprecated and falls back to fetchCachedValueFromSecondary Fixes: SIRI-1039
Fixes: SIRI-1039
Fixes: SIRI-1039
idlira
added
🧬 Enhancement
since we cannot guarantee the order in which tests are executed, we strip a portion of the expected name (so matching "Parent 3" or "Parent 3.1") Fixes: SIRI-1039
| @@ -31,24 +31,24 @@ class SmartQueryKotlinTest { | |||
| @Test | |||
| fun `queryList returns all entities`() { | |||
| val smartQueryTestEntity = oma.select(SmartQueryTestEntity::class.java) | |||
| .orderAsc(SmartQueryTestEntity.TEST_NUMBER) | |||
| .orderAsc(SmartQueryTestEntity.TEST_NUMBER) | |||
There was a problem hiding this comment.
Looks as if we need to optimise our Kotlin formatting rules 😇
There was a problem hiding this comment.
They are (or rather should be when the IDE is configured correctly) defined by the Kotlin language themselves. It is highly discouraged to stray away from it and create a customized formatting ruleset.
But this formatting looks like the language default isn't applied here.
Maybe we can change something in sirius-parent to make usage easier.
| assertEquals(listOf("Parent 2"), result.stream().map { x -> x.name }.collect(Collectors.toList())) | ||
| assertEquals( | ||
| listOf("Parent 2", "Parent 3"), | ||
| result.stream().map { x -> x.name.substring(0, 8) }.collect(Collectors.toList()) |
There was a problem hiding this comment.
- Add a comment explaining this?
- Protect against
x.name.length< 8? - Compare sets (or sorted lists) if we cannot guarantee the order?
There was a problem hiding this comment.
I've added a comment about why we substring.
For the other comments: The data used in these test cases are inserted during the class setup, therefore controlled data, which means:
x.name.length < 8cannot happen- The order of items will be guaranteed by the ID (they are inserted in order). What we cannot guarantee is the oder in which tests are executed.
There was a problem hiding this comment.
isolated tests would be great :-)
Fixes: SIRI-1039
| @@ -31,24 +31,24 @@ class SmartQueryKotlinTest { | |||
| @Test | |||
| fun `queryList returns all entities`() { | |||
| val smartQueryTestEntity = oma.select(SmartQueryTestEntity::class.java) | |||
| .orderAsc(SmartQueryTestEntity.TEST_NUMBER) | |||
| .orderAsc(SmartQueryTestEntity.TEST_NUMBER) | |||
There was a problem hiding this comment.
They are (or rather should be when the IDE is configured correctly) defined by the Kotlin language themselves. It is highly discouraged to stray away from it and create a customized formatting ruleset.
But this formatting looks like the language default isn't applied here.
Maybe we can change something in sirius-parent to make usage easier.
| assertEquals(listOf("Parent 2"), result.stream().map { x -> x.name }.collect(Collectors.toList())) | ||
| assertEquals( | ||
| listOf("Parent 2", "Parent 3"), | ||
| result.stream().map { x -> x.name.substring(0, 8) }.collect(Collectors.toList()) |
There was a problem hiding this comment.
isolated tests would be great :-)
Moves the actual lookup to the "forced" methods, making the calls a bit more natural Fixes: SIRI-1039
Description
Always use the ID stored in the reference to retrieve an item from the cache. Previously, if the reference was pre-loaded, that value was delivered instead.
fetchValue()is now deprecated and replaced byfetchCachedValue()+forceFetchValue(). Same for the*FromSecondaryvariants.Marked as BREAKING since usages of fetchValue() and fetchValueFromSecondary() in tagliatelle templates might lead to build warnings.
To keep the previous semantics, simply replace
fetchValue()byfetchCachedValue()andfetchValueFromSecondary()byfetchCachedValueFromSecondary()Additional Notes
Checklist