Skip to content
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

Added schema validation on context startup #1466

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mipo256
Copy link
Contributor

@mipo256 mipo256 commented Jan 4, 2024

This is the PR to solve #764 issue.
The key takeaways in the PR: there is a new class, called CassandraSchemaValidator, that is validating Cassandra schema against existing entities in the MappingContext. The keyspace where we expect the entities to show up is one that is configured to be used by CqlSession (for now that is correct because we do not have a keyspace segregation for entities yet, see this ticket).

The validation checks that:

  • All Tables exists
  • All non-transient properties are expected to be mapped to a single column (I do not know if this is correct or not, appreciate any help on this)
  • We expect that the type of the persistent property matches the one we expect in Cassandra

P.S: I think it would be nice to add this as a bean to Cassandra auto-configuration to spring boot with some properties conditional annotations

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 4, 2024
@mp911de
Copy link
Member

mp911de commented Jan 9, 2024

Thanks for having a look. The design requires a bit more thought and not everything should be a String or Exception. Design-wise, we should decouple validation from any factory beans for a clearer separation of concerns. Later on, once the validation part is fine, we can consider how to integrate validation into application startup.

For type resolution, I suggest you have a look at DefaultColumnTypeResolver and SchemaFactory, how data types are derived from properties.

Also, validation findings should be part of a model (ColumnNotFound, TableNotFound) along with references to the underlying CQL item/persistent property/persistent entity.

Let me know whether that helps.

@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 9, 2024
@mipo256
Copy link
Contributor Author

mipo256 commented Jan 9, 2024

@mp911de I have introduced a couple of changes in accordance to what you've said:

  1. CassandraSchemaValidator now accepts CqlSession directly instead of CqlSessionFactoryBean
  2. CassandraSchemaValidator now also accepts CassandraConverter instead of CassandraMappingContext. This allwos getting CassandraMappingContext from CassandraConverter and also the creation of DefaultColumnTypeResolver instance using CassandraConverter`

But I do not fully understand wht you mean here:

Also, validation findings should be part of a model (ColumnNotFound, TableNotFound) along with references to the underlying CQL item/persistent property/persistent entity.

Can you please give me a glue what is your vision here? You want CassandraSchemaValidationProfile to accept not String objects as validation error messages but have an abstraction over a message, such as ColumnNotFound and TableNotFound and so on, am I correct?

@mipo256
Copy link
Contributor Author

mipo256 commented Mar 5, 2024

I have rebased this PR. Any chance to review it? @mp911de

@mp911de
Copy link
Member

mp911de commented Mar 5, 2024

Currently, this ticket (and several other ones) is on pause because we're limited in bandwidth. We currently can only fix smaller bugs and I don't know when we will have more time for proper enhancements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants