Skip to content

Conversation

alexander-yevsyukov
Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov commented Mar 21, 2017

This PR implements CommandStore as a Repository.

Breaking changes:

  • The Repository now does not have BoundedContext constructor argument. AggregatePartRepository, ProcessManagerRepository, and ProjectionRepository still have such a constructor argument.
  • CommandStore creates its storage with the help of StorageFactory for creating CommandStore as a repository.
  • Because of the above, StorageFactory does not produce CommandStorage any more.
  • Because of the above, InMemoryCommandStorage class was also removed.

…ository

Conflicts:
	server/src/main/java/org/spine3/server/command/CommandStorage.java
Conflicts:
	server/src/main/java/org/spine3/server/entity/AbstractEntity.java
Also:
 * Removed `BoundedContext` ctor parameter required for `Repository` and made it where dependency on a `BoundedContext` is really needed.
 * Removed `CommandStorage` as an abstract class that needs to be implemented and created by a `StorageFactory`.
Also removed `@SPI` annotation from `CommandStore`, which is package-private can is not SPI because of this.
@alexander-yevsyukov alexander-yevsyukov self-assigned this Mar 21, 2017
@codecov
Copy link

codecov bot commented Mar 21, 2017

Codecov Report

Merging #394 into master will increase coverage by 0.01%.
The diff coverage is 93.38%.

@@             Coverage Diff              @@
##             master     #394      +/-   ##
============================================
+ Coverage     92.86%   92.88%   +0.01%     
- Complexity     2352     2355       +3     
============================================
  Files           230      229       -1     
  Lines          7516     7532      +16     
  Branches        541      543       +2     
============================================
+ Hits           6980     6996      +16     
- Misses          411      414       +3     
+ Partials        125      122       -3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cbd89f...fb4e049. Read the comment docs.

@alexander-yevsyukov
Copy link
Contributor Author

@armiol, PTAL.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexander-yevsyukov LGTM except for the Javadoc wording issues. Please take a look at those before merging.

* @param status
* a command status to set in the record
* @param generatedCommandId
* a command ID to used because the passed command does not have own ID.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ... to be used... or ... to use ....

* a command status to set in the record
* @param generatedCommandId
* a command ID to used because the passed command does not have own ID.
* If the command has own ID this parameter is {@code null}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please insert a comma after has own ID.

* But this ID does not belong to the command.
*
* <p>Therefore, commands without ID can be found by records
* where `command.context.command_id` field is empty.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use {@code command.context.command_id} instead of backquotes?


/** The repository completed the catch-up process. */
ONLINE,
/** If {@code true} the projection will {@link #catchUp()} after initialization. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add an alternative text to this {@link #catchUp()}.

@alexander-yevsyukov alexander-yevsyukov merged commit ee5ad60 into master Mar 21, 2017
@alexander-yevsyukov alexander-yevsyukov deleted the command-repository branch March 21, 2017 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants