-
Notifications
You must be signed in to change notification settings - Fork 372
DATAJDBC-222 - Full editing pass for spring-data-jdbc #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I edited for spelling, punctuation, grammar, usage, and corporate voice. I also added an epub cover image and the SVG file from which it can be generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at my comments.
src/main/asciidoc/jdbc.adoc
Outdated
Also, things that are really simple conceptually get rather difficult with JPA. | ||
|
||
Spring Data JDBC aims to be much simpler conceptually: | ||
Spring Data JDBC aims to be much simpler conceptually, by supporting the following features: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"supporting the following features" doesn't sound right to me. In many cases it is more the lack of features. Maybe "by following these design decisions:"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with "by embracing the following design decisions:" because I need to use "following" to lead into the list.
src/main/asciidoc/jdbc.adoc
Outdated
An aggregate is a group of entities that is guaranteed to be consistent between atomic changes to it. | ||
A classic example is an `Order` with `OrderItems`. | ||
A property on `Order`, e.g. `numberOfItems` will be consistent with the actual number of `OrderItems`. | ||
A property on `Order` (for example, `numberOfItems`) is consistent with the actual number of `OrderItems`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the example does not end with numberOfItems
but goes all the way to the end of the sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes a sentence fragment, so I had to extend the sentence there.
|
||
All Spring Data modules are inspired by the concepts of Repository, Aggregate and Aggregate Root from Domain Driven Design. | ||
These are possibly even more important for Spring Data JDBC because they are to some extend contrary to normal practice when working with relational databases. | ||
All Spring Data modules are inspired by the concepts of "`repository`", "`aggregate`", and "`aggregate`" root from Domain Driven Design. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think "repository", "aggregate" and "aggregate root" should be formatted as code since they are theoretical concepts/ patterns.
And the last one should be "aggregate root" (root being part of the pattern name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The back ticks inside the double quotation marks render as curly quotation marks. They won't show up as code. It's an oddity of Asciidoc. I'm trying to consistently use curly quotation marks in our output.
If the aggregate is new, this results in an insert for the aggregate root, followed by insert statements for all directly or indirectly referenced entities. | ||
|
||
If the Aggregate Root is _not new_ all referenced entities will get deleted, the Aggregate Root updated and all referenced entities will get inserted again. | ||
If the aggregate root is not new, all referenced entities get deleted, the aggregate root gets updated, and all referenced entities get inserted again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"new" is a specific state the instance is in. This might be clear from context but I wonder if there is an established way to make that clear. It's what I tried with the emphasis in the original version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the context makes that clear. It did for me. I added a sentence to make it absolutely clear, though.
One important constraint is that after saving an entity the entity must not be _new_ anymore. | ||
With autoincrement columns this happens automatically since the the id gets set by Spring Data with the value from the id column. | ||
If you are not using autoincrement columns you can use that using a `BeforeSave`-listener which sets the id of the entity (see below). | ||
One important constraint is that, after saving an entity, the entity must not be new any more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a sentence to make it absolutely clear.
src/main/asciidoc/jdbc.adoc
Outdated
With autoincrement columns this happens automatically since the the id gets set by Spring Data with the value from the id column. | ||
If you are not using autoincrement columns you can use that using a `BeforeSave`-listener which sets the id of the entity (see below). | ||
One important constraint is that, after saving an entity, the entity must not be new any more. | ||
With auto-increment columns, this happens automatically, because the ID gets set by Spring Data with the value from the `id` column. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if id
is the correct formatting, I think it should be "ID column" as two lines above, since it is the identifying column, not necessarily the column named id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I changed it.
src/main/asciidoc/jdbc.adoc
Outdated
If you are not using autoincrement columns you can use that using a `BeforeSave`-listener which sets the id of the entity (see below). | ||
One important constraint is that, after saving an entity, the entity must not be new any more. | ||
With auto-increment columns, this happens automatically, because the ID gets set by Spring Data with the value from the `id` column. | ||
If you are not using auto-increment columns, you can use a `BeforeSave` listener, which sets the `id` of the entity (covered later in this document). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the same argument, this should be "ID"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup
|
||
[[jdbc.query-methods.at-query]] | ||
=== Using @Query | ||
=== Using `@Query` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the rule no code formatting in headings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put code formatting in headings. I have observed that it works correctly in cross-references and the ToC, and I want the consistency with the body. (I gather that Asciidoc used to have trouble with it, but now it works.)
Jens reviewed my PR and had some feedback. I either fixed things or explained why I made the choice I made. Thanks for the review, Jens. :)
Edited for spelling, punctuation, grammar, usage, and corporate voice. Also added an epub cover image and the SVG file from which it can be generated. Original pull request: #73.
That's merged onto master. Thanks. |
We now encapsulate prepared operations from the StatementFactory within PreparedOperation that renders SQL and provides binding values. StatementFactory supports SELECT/INSERT/UPDATE/DELETE statement creation considering Dialect-specific rendering. StatementFactory replaces String-based statement methods in ReactiveDataAccessStrategy. PreparedOperation<Update> operation = accessStrategy.getStatements().update(entity.getTableName(), binder -> { binder.bind("name", "updated value"); binder.filterBy("id", SettableValue.from(42)); }); databaseClient.execute().sql(operation).then(); Original pull request: #82.
We now apply bindings to a BindTarget that can be overridden without the need to implement all Statement methods. PreparedOperation no longer has a direct dependency on R2DBC Statement. Original pull request: #82.
I edited for spelling, punctuation, grammar, usage, and corporate voice. I also added an epub cover image and the SVG file from which it can be generated.