> Would the individual commands register via classpath scanning or static > initializers or is this something that @Endpoint already does for us?
A bit of both as I understand it. Classes containing these endpoint methods must be explicitly registered in CoreContainer, but from there reflection is used to pick up all endpoints in that class. I'm all for David's idea of splitting CollectionsHandler up into files that are organized 1-endpoint-per-file. I'd even be on board with keeping related endpoints/commands in the same class but organized by different methods. (You see this in many projects today that use JAX-RS or other REST annotation frameworks, and I've always found those to be navigable and understandable). Either one seems preferable to having these methods that operate on huge, complex enums, as CollectionsHandler does today. Best, Jason On Thu, Apr 29, 2021 at 5:50 PM Mike Drob <[email protected]> wrote: > > Would the individual commands register via classpath scanning or static > initializers or is this something that @Endpoint already does for us? In > general, I would be in favor of splitting out the commands, although some of > them might still make sense to group together. > > On Thu, Apr 29, 2021 at 4:05 PM David Smiley <[email protected]> wrote: >> >> 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 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
