Make Storage API and implementation more friendly to Spine's storage-specific libraries #1533
Conversation
|
@alexander-yevsyukov PTAL. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1533 +/- ##
============================================
- Coverage 89.90% 89.83% -0.08%
+ Complexity 5057 5025 -32
============================================
Files 648 645 -3
Lines 15806 15751 -55
Branches 925 920 -5
============================================
- Hits 14211 14150 -61
- Misses 1269 1274 +5
- Partials 326 327 +1 |
…'s history. See [jdbc-storage#155] on why this matters.
…used in Spine storage libraries.
armiol
changed the title
armiol
changed the title
armiol
changed the title
# Conflicts: # license-report.md
…dy has its ID with it. Address minor documentation issues as well.
# Conflicts: # license-report.md # pom.xml # version.gradle.kts
…vor of `MessageRecordSpec` and `RecordWithColumns`.
Document the difference between `recordType()` and `sourceType()`. Rename `MessageRecordSpecTest.java` into `RecordSpecTest.java`.
# Conflicts: # license-report.md # pom.xml # server/src/main/java/io/spine/server/stand/Stand.java # version.gradle.kts
# Conflicts: # license-report.md # pom.xml # version.gradle.kts
|
@alexander-yevsyukov it has been a long time since this PR was first approved. Many things have changed since then. PTAL again. |
| * @param <S> | ||
| * type of entity state | ||
| */ | ||
| private static final class MemoizingUnpacker<I, S extends EntityState<I>> { |
There was a problem hiding this comment.
Please move this class to the bottom of the outer one.
| */ | ||
| @SPI |
There was a problem hiding this comment.
Assuming that the whole package is annotated as @SPI, the classes and interfaces that were annotated directly should lose the redundant annotation too. E.g. ColumnMapping, and ColumnTypeMagging are currently annotated directly.
Please have a look.
There was a problem hiding this comment.
Or, consider removing the package-wide annotation and use only annotation on classes and interfaces.
There was a problem hiding this comment.
I have removed @SPI from package members in favour of package-level annotation.
| @@ -89,8 +89,5 @@ message MirrorId { | |||
| // Entity columns of the associated aggregate. | |||
| message EntityColumns { | |||
|
|
|||
| // TODO:2018-09-06:dmytro.dashenkov: Support entity columns. | |||
|
@alexander-yevsyukov while reviewing the And considering |
|
@alexander-yevsyukov PTAL. |
This PR addresses some inconsistencies in both Storage API and its base/abstract implementation, so that descendant port libraries, such as Spine RDBMS could migrate their features to Spine 2.x with less hassle.
Major breaking change (breaking for SPI users, that is) is merging
EntityRecordSpecandMessageRecordSpecinto a singleRecordSpectype. It has been done for several reasons.EntityRecords consistently on top ofEntityRecorditself. Previously, the column values inEntityRecordWithColumns— which were picked fromEntitystate directly — could differ from those values actually stored inEntityRecord.RecordWithColumnsinstances purely with the relevantMessageinstance along with its spec. Previously,Entity-relatedRecordWithColumnscould only be assembled with anEntityinstance.Other changes include:
public.RecordQueryinstances now preserves the type of the column, by which values query aims to filter the dataset.AggregateStorage.historyBackward()has been made eager instead of lazy, as previously. This is needed to ensure the access to the history under the circumstances of maintaining the connection to the database.InboxStorage).FieldMaskApplierhas been made@Internal-lypublic, as its functionality is required in descendant libraries to prevent any code duplication.