Isolate shared RecordStreamSource functionality #193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
saig0
approved these changes
Feb 22, 2022
There was a problem hiding this comment.
@remcowesterhoud looks good 🚀
I have just a small suggestion. Please have a look before merging.
filters/src/main/java/io/camunda/zeebe/process/test/filters/RecordStream.java
Outdated
Show resolved
Hide resolved
Creates a new RecordStream class which contains the common functionality that was part of the RecordStreamSourceImpl. This is necessary because we will have different implementations of RecordSource and we don't want to implement these shared functions between all of them.
Removes the shared functionality from the RecordStreamSource. These functionalities have been moved to RecordStream. RecordStreamSource is only responsible for providing records (it is the source of the records).
The engine will no longer have he option to filter records by default in the RecordStreamSource. Because of this we need to wrap the RecordStreamSource into a RecordStream in places we want to use these filters. For this a dependency on the filters module is required.
…ource The functionality used by the filters was removed from RecordStreamSource and placed in the new RecordStream class. Therefore, the filters should now use a RecordStream instead of a RecordStreamSource.
Places that used a RecordStreamSource before should now be using the new RecordStream.
This constructor is never accessed outside of this class. Other classes all use RecordStream.of().
remcowesterhoud
force-pushed
the
separate-recordstream
branch
from
e800ca8 to
e388110
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The
RecordStreamSourceinterface defines a lot of methods that generic and should be reusable. This PR creates a newRecordStreamclass in thefiltersmodule. This class will implement these generic functions, so not allRecordStreamSourceclasses have to implement them separately, preventing duplication.It's quite a refactoring so I've done it in a few steps to make it easier to review:
RecordStreamclass containing the shared functionality.RecordStreamSourceinterface and the implementation.RecordStreamclass in theenginemodule.filtersmodule use the newRecordStreamclass.RecordStreamSourceto the newRecordStreamin the other modules.In Zeebe we have a similar functionality in the
RecordingExporter. It would be nice if we could share this functionality between the 2 projects at some time. We can't use the zeebe implementation at the moment because it is not Java 8 compatible.Related issues
This is a requirement for #192 . Without doing this #192 would introduce a lot of duplication as it needs create an implementation of the
RecordStreamSourceinterface.Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
Testing:
Documentation: