what do you think of the idea of adding an on: :create validation to the Dependency model?

@segiddins Just took a look - Could be off but my understanding is that the on create validation in Dependency would have to prevent the push if the name is unresolved. It seems like for other contexts(not pushing), there is a validation already called use_existing_rubygem but it just sets the unresolved_name of the dependency if its not found. If we were to go this route, would it make sense to add a condition into this validation to check whether a gem is being pushed rather than creating a new validation?

I'm leaning towards having it in Pusher because it checks all dependencies in one query and can fail early before creating Dependency objects, which seems more efficient than adding per-dependency validations in the context of pushing. I could totally be missing something though - what do you think?

which seems more efficient than adding per-dependency validations in the context of pushing

We're already doing the query though to find the rubygem in Dependency#use_existing_rubygem. Adding a validation on: :create wouldn't require any additional queries. Version#update_dependencies! is already doing the lookup one-by-one

Since use_existing_rubygem is a before_validation hook, I think we would want a new validation, so it becomes easy to scope to on: :create, but writing one that gives an error message saying which gem name isn't resolved seems both easy and a good idea

@segiddins
Ah I see, I think I was mistaken in thinking that the additional Dependency validation would add a query for each dependency. If I'm understanding correctly now, you're saying the new validation wouldn't create extra queries since it would use the results already found by use_existing_rubygem?

Also wanted to circle back to my point about early validation in Pusher. Say a gem being pushed has 30 dependencies and the last one is invalid, was thinking the validation in Dependency would do something like this:

1. Create 30 Dependency objects
2. Run use_existing_rubygem 30 times (30 queries)
3. Fail on the last validation

While the Pusher validation would go something like this:

1. Run one query to check for and return all missing dependencies
2. Fail early since last one is invalid
3. Skip creating any Dependency objects since validation fails (no additional queries)

What are your thoughts on this aspect? I do agree the validation fits more naturally in the Dependency model, but it seems like the early check could save some work. I'm not sure how significant the savings would be in practice though, perhaps a dependency typo is rare event not worth worrying about