Skip to content

#30 - Add custom conversion support to write array-typed values. #31

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

Closed
wants to merge 6 commits into from

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Dec 3, 2018

We now support custom conversions via R2dbcCustomConversions. Custom conversions introduces simple types that depend on the used dialect. Custom conversions and simple types are held in RelationalConverter and MappingContext.

Simple types and conversions are used by DatabaseClient and repository support to properly apply registered converters and support native types such as array-columns.


Supersedes PR #22.
Related tickets: #22, #26, #30.

@mp911de mp911de requested a review from schauder December 3, 2018 16:45
@schauder schauder self-assigned this Dec 4, 2018
rdegnan and others added 4 commits December 4, 2018 10:22
EntityRowMapper now passes-thru values for Collection-like types such as array. Arrays are supported by Postgres.

Original pull request: #22.
Add author tag. Add unit test for EntityRowMapper.
We now support custom conversions via R2dbcCustomConversions. Custom conversions introduces simple types that depend on the used dialect. Custom conversions and simple types are held in RelationalConverter and MappingContext.

Simple types and conversions are used by DatabaseClient and repository support to properly apply registered converters and support native types such as array-columns.

Related tickets: #22, #26.
Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

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

I think I found a couple of things that should work, but don't.

I'd also like to see some integration tests for the database(s) that support arrays.

Minor formatting. Add suggestions.

Update src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java

Co-Authored-By: mp911de <[email protected]>
@mp911de
Copy link
Member Author

mp911de commented Dec 5, 2018

I applied your suggestions @schauder and pushed the improvements.

Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

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

I think I found one more corner case that should get proper handling.

Plus small stuff.

ArrayColumns arrayColumns = PostgresDialect.INSTANCE.getArraySupport();

assertThatThrownBy(() -> arrayColumns.getArrayType(getClass())).isInstanceOf(IllegalArgumentException.class);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}
@Test // gh-30
public void shouldRejectNestedCollections() {
ArrayColumns arrayColumns = PostgresDialect.INSTANCE.getArraySupport();
assertThatThrownBy(
() -> arrayColumns.getArrayType(List.class)
).isInstanceOf(IllegalArgumentException.class);
}

The added test fails but I think it should because we don't apply the conversion to arrays recursively.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add the test and remove List from simple types. While Postgres supports n-dimensional array, the driver does not support that option so it's safe for now to only support 1-dimensional arrays without recursion.

Introduce ArrayColumns type to encapsulate Dialect-specific array support. Apply array conversion for properties that do not match the native array type. Add integration tests for Postgres array columns.
schauder pushed a commit that referenced this pull request Dec 7, 2018
Add author tag. Add unit test for EntityRowMapper.

Original pull request: #31.
schauder pushed a commit that referenced this pull request Dec 7, 2018
We now support custom conversions via R2dbcCustomConversions.
Custom conversions introduces simple types that depend on the used dialect.
Custom conversions and simple types are held in RelationalConverter and MappingContext.

Simple types and conversions are used by DatabaseClient and repository support to properly apply registered converters and support native types such as array-columns.

Related tickets: #22, #26.
Original pull request: #31.
schauder added a commit that referenced this pull request Dec 7, 2018
Minor formatting. Add suggestions.

Update src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java

Co-Authored-By: mp911de <[email protected]>

Original pull request: #31.
schauder pushed a commit that referenced this pull request Dec 7, 2018
Introduce ArrayColumns type to encapsulate Dialect-specific array support.
Apply array conversion for properties that do not match the native array type.
Add integration tests for Postgres array columns.

Original pull request: #31.
@schauder
Copy link
Contributor

schauder commented Dec 7, 2018

Merged.

@schauder schauder closed this Dec 7, 2018
@schauder schauder deleted the issue/gh-30 branch December 7, 2018 10:58
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.

3 participants