For a long time, I have not been fond of how the code for our collection APIs are organized. I would prefer to see more separation between the commands (thus no long class files implementing many commands), and fewer places to touch generally. If more of a command's logic is in its own source file, I think this would be easier to understand (less indirection across many classes), easier to code review, and easier to observe from a source control perspective.
I just did a review of "MODIFYCOLLECTION" to illustrate the situation and also because Jason was looking at this one in SOLR-15351: Solr-Core: * collections.collection.Commands.modify.json (43 lines of metadata) * CollectionsHandler: CollectionOperation enum entry (~20 lines) * Overseer: processMessage switch entry (2 lines, trivial code) * DistributedClusterStateUpdateer: MutatingCommand enum entry (6 lines, trivial code) * CollectionMutator: (55 lines) SolrJ: * CollectionParams: CollectionAction enum entry (one line, trivial code) * CollectionApiMapping: Meta enum (~4 lines, trivial code) * CollectionAdminRequest: convenience method & class for users/clients (~80 lines) Furthermore, in SOLR-15351, Jason has a proposal to add another class with per-command code (~30 lines for this command) that would replace the modify.json one above. I *really* like the essence of SOLR-15351 but I want to discuss on the dev list how we organize our code for these admin API commands. Imagine a "ModifyCollectionApi" class file. It would have a method with an @EndPoint annotation that Jason shows in SOLR-15351. Unlike the current state of that PR, it could also absorb more than that. Let's have it absorb the logic in CollectionsHandler for this command. Assuming we fully implement this approach, it would mean CollectionsHandler would be massively gutted and wouldn't be on the list above. That separation would be nice IMO but there are still many places to touch to understand how this one command works. After this stage, it's worth seeing if it's best to have ModifyCollectionApi also contain the CollectionMutator logic for this command. Probably? Some of the APIs have differing backends to their implementation. The Core admin APIs could be improved to benefit from "@EndPoint" but the situation there isn't quite so bad ever since "CoreAdminOp" came into being with the separation of each implementation into separate source files that occurred in SOLR-9523 in 2016. ~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley
