Skip to content

Conversation

fabiojmendes
Copy link
Contributor

Hi There!

I spent the weekend working on DATACASS-172 and as this is a feature with a considerable amount of work I wanted to start an early feedback loop.

I implemented the basic save and retrieve scenarios and also did some changes to the validation to support the @UserDefinedType annotation. Right now, my biggest concern is the circular referrence I created at the BasicCassandraMappingContext (line 304). I need some way to access the user defined type metadata but the CassandraAdminOperations depends on the mappingContext.

Going forward there are some improvements to be done to the whole ColumnReader, UDTValueReader/Writer thing, schema generation might be tricky because now you can create dependencies between user types, collections of UDTs and querying.

Looking forward to hear from you.

Fabio

@fabiojmendes fabiojmendes changed the title Datacass 172 DATACASS-172 How to handle CUSTOM User Defined TYPEs Mar 17, 2015
@mp911de
Copy link
Member

mp911de commented Aug 29, 2016

Hi @fabiojmendes
I'm sorry it took so long to come back to you and that no one else got in touch with you since you opened this PR. Are you still interested in continuing that PR?

@fabiojmendes
Copy link
Contributor Author

fabiojmendes commented Sep 5, 2016

Hey Mark, it was indeed very frustrating that no one even bother looking at a feature that I think is rather important to the framework. Even if it was to reply a flat NO, that wasn't the way to get things done.

Let me have a look, I'll try to merge with the current master branch. I deleted my fork so I don't know if there's a way to get the changes from this PR back without some nasty copying and pasting.

And BTW, did you get a chance to review it? Was I on the right track?

@mp911de
Copy link
Member

mp911de commented Sep 6, 2016

I'm sorry again how this PR was treated. You were on a good track. The @UserDefinedType annotation fits well. The CQL generators/specifications are in good shape. The other parts, like object mapping, were changed in the meantime a lot, that's why you see all these merge conflicts.

I started importing your changes (see https://github.com/mp911de/spring-data-cassandra/commits/issue/DATACASS-172) from the PR. You can view a PR also as patch file (https://patch-diff.githubusercontent.com/raw/spring-projects/spring-data-cassandra/pull/32.patch) which makes things a bit easier.

MappingContext (CassandraPersistentEntity, CassandraPersistentProperty) are challenging since we don't access Cassandra inside of them. That is necessary to obtain the UserType. Our EntityVerifier undergoes right now also a change that is required to be finished before we can proceed with UDT support to not reject UDT types.

It was important for me to come back to you as you did a fantastic job by digging into Spring Data Cassandra and building a great feature.

@fabiojmendes
Copy link
Contributor Author

fabiojmendes commented Sep 7, 2016

No worries, thanks for getting back to me. I'm glad that the changes are still relevant to the project.

I was going through your latest code check in and the structure is very different from the one I worked on as you said, I'm having a hard time getting up to speed with it actually.

Looks like you've got the mapping figured out, is there anything I can help you with?

@mp911de
Copy link
Member

mp911de commented Sep 7, 2016

You helped a lot to get up to speed with that particular feature. You provided a lot of valueable input. Spring Data Cassandra evolved in the last 6 months so we moved things around.

Having another pair of eyes on the code is always helpful. I will open a PR soon (that's the way we work with Spring Data) that will contain your changes and mine. If you want, give that code a try from an users' perspective and feel free to comment on the code.

@mp911de
Copy link
Member

mp911de commented Sep 12, 2016

I created #84 that contains your and my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants