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

Validations #759

Open
harjis opened this issue Jan 7, 2020 · 4 comments
Open

Validations #759

harjis opened this issue Jan 7, 2020 · 4 comments

Comments

@harjis
Copy link

harjis commented Jan 7, 2020

Hi,

I have been trying out Exposed for a while and I noticed that there is no built in support for validations either on DSL or DAO side. I suppose this is by design if Exposed is a lightweight SQL library. I can see the argument on not having validations in exposed. The reality however is that I have had the need for validations in every project I have been involved with.

If exposed is not going to have support for validations what do you think would be the best approach to deal with them on application level both on DSL and DAO style? One approach could be adding them to the repository:

DAO:

object Events : IntIdTable() {
    val startDate = date("start_date")
    val endDate = date("end_date")
}

class Event(id: EntityID<Int>) : IntEntity(id) {
    companion object : IntEntityClass<Event>(Events)

    var startDate by Events.startDate
    var endDate by Events.endDate
}

data class EventDataClass(val startDate: DateTime, val endDate: DateTime)

class EventsRepository {
    fun save(event: EventDataClass) {
        val errors = validate(event)
        if (errors.isNotEmpty()) {
            throw Exception(errors.toString())
        }

        Event.new {
            startDate = event.startDate
            endDate = event.endDate
        }
    }

    private fun validate(event: EventDataClass): MutableList<String> {
        val errors = mutableListOf<String>()
        if (event.endDate.isBefore(event.startDate)) {
            errors.add("End date is before start date")
        }

        return errors
    }
}

For a developer that comes from Ruby on Rails world this code looks a bit odd. My expectation was that Event class would be the public API to create new records in DB but at least if I want to add validations it looks like it can't be the public API. Also all() method is not transactional so is the design idea here indeed so that in both DSL and DAO styles a repository should be implemented which is transactional?

Second question is also kind of related on validations. Is it so that there is no support for single table inheritance? In Rails what I could do is add validations but also relationships on the child models:

class Calendar < ApplicationRecord
  has_many :events
end

class Event < ApplicationRecord
  belongs_to :calendar
end

class PrivateEvent < Event
  belongs_to :user
end

class PublicEvent < Event

end

calendar = Calendar.create()
public_event = PublicEvent.create(calendar: calendar) # OK

calendar = Calendar.create()
private_event = PrivateEvent.create(calendar: calendar) # NOT OK: User is missing
puts private_event.errors # Prints out a validation error that user is required.

If there is no support in Exposed for this, ideas to implement this on application level are welcome too.

@harjis
Copy link
Author

harjis commented Jan 9, 2020

I played around with the code a bit more and I think I might be a bit closer in understanding my confusion.

So like mentioned I have a longish background with Rails so I tend to compare functionality to it quite often (which might or might not make sense). It seems that there is a difference between .all method in rails and .all() method in Exposed. In rails transaction is not opened nor is it required when .all is called. In Exposed (and to my recollection other JVM ORM libraries like Hibernate actually hibernate does not require transaction in db reads) you are required to open a transaction if you want to read from the db. I wonder why there is a difference like this? I suppose the difference is there because in rails .save is wrapped automatically in a transaction and committed also but in Exposed .new {} does not do that, right? For this reason we need to flush entitites in db before any queries are made.

@Tapac
Copy link
Contributor

Tapac commented Jan 10, 2020

@harjis , I agree that validation on DSL level will be a nice feature.

About .all() function: it doesn't require transaction until you start iterating over its result.

About new{} and flushing: it works the same way as a changing of an existing entity (see here). Moreover, if you create an entity with new{} and then modify it before any other queries, changes will be accumulated and only a single insert will be executed.

We are going to release updated documentation later this year where this will be covered.

@harjis
Copy link
Author

harjis commented Jan 14, 2020

My point about .all() method is that in those frameworks I have used repository or the activerecord class can be considered as public API for the table.

Hibernate:

class PostController(private val postRepository) {
    fun index() = postRepository.findAll()
}

Rails:

class PostController < ApplicationController
    def index
       @posts = Post.all
    end
end

Even though exposed DAO API looks very similar to Rails ActiveRercord one you can not use it the same way:

data class PostView
class PostController {
    fun index() = Post.all().map { PostView() } // Not OK because transaction has not been opened
}

In order to make this to work I see 2 options. Either open a transaction in controller:

class PostController {
    fun index() = transaction { Post.all().map { PostView() } }
}

Or create a repository class which opens the transaction:

data class PostView
class PostRepository {
    fun all() = transaction { Post.all() }
}
class PostController(private val postRepository) {
    fun all() = postRepository.all().map { PostView() }
}

Argument against opening transaction in controller is that in MVC controller should not know that much about the underlying persistence layer as that responsibility should be on the model.

For me the repository one is a bit confusing as the exposed DAO API is so close Rails ActiveRercord one that I thought that should be the public API for the table but actually you can not use it the same way.

My comments regarding .new {} and .all() were related to this. As this is old version of the code I think we can disregard the comments for now.

@yassenb
Copy link

yassenb commented Jan 25, 2023

Any update on this? Also supporting lifecycle hooks like "before save" etc. would be a way to hook in an existing validation library but those hooks don't exist either

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

No branches or pull requests

3 participants