Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
@AndyPook
Good point on the `context` parameter. After [taking a closer
look](https://github.com/apache/lucene-solr/blob/releases/lucene-solr/4.8.1/lucene/replicator/src/java/org/apache/lucene/replicator/http/HttpClientBase.java#L77-L78),
there is a `path` parameter on the client. Why they didn't name it `context`
to match the server is beyond me, but it appears to be one and the same. So,
effectively, we can get rid of that parameter and replace it with `urlPrefix`
because they are the same thing. And because we have a prefix (and indeed it is
not optional), we don't need a route constraint.
Funny how long it is taking us to work this out since they admitted on that
blog post that they built replicator in just 1 day. But they certainly could
have documented it better in the API docs instead of falling back on a blog to
serve as the documentation.
Actually, the above code wasn't intended to be a finished product, but a
demonstration of how to fit the pieces together. And making the pieces fit
first (and tests pass) before refactoring is a good strategy. One decision that
I will probably end up changing is to make the parameter `IReadOnlyDictionary`
instead of `IDictionary`, since we don't want anyone tinkering with the
contents of this singleton after application startup.
While I agree with you that the switch case statement in
`ReplicationService` seems to be screaming out for refactoring, I think Jens is
also right on the money on this approach. Being that there needs to be some
server-side interaction between the runtime code and HTTP listener, it is not
possible to just wrap this up into a generic service that can be installed with
an installer. In fact, we should build similar integration packages for MVC 5,
WebApi2, and possibly even web forms. And if we are doing that, we should wait
to see how we can create those integration packages before refactoring
`ReplicationService` (assuming refactoring it is even in the cards - it may
ultimately prove to be more useful to serve as the generic "servlet" piece that
is missing from .NET). I think what Jens did with the `IReplicationRequest` and
`IReplicationResponse` abstractions is a fine idea that will ultimately prove
useful for plugging into each of these frameworks. And if `ReplicationService` i
s something the user never has to deal with anyway, maybe it doesn't make
sense to change it as tempting as that might be. After all, if we refactor it,
then we have to also refactor the tests.
@jeme
I am not implying you have to do all of the work to integrate with each of
those frameworks, just finishing AspNetCore is enough. Actually, integrating
with the other frameworks will be a bit more tricky because the lack of default
DI container, so I the most intuitive approach might be to register the
dictionary statically on those.
I have arrived at a decision on a folder structure for the .NET wrapper
projects. We should put `Lucene.Net.Replicator.AspNetCore` into a new directory
`src/dotnet/`. This will be the place where all of the projects go that we
aren't specifically porting from Java, and eventually we can move the
`Lucene.Net.ICU` and `lucene-cli` projects and their tests there, too. However,
if you want to give this new project a permanent home, go ahead and create that
folder as part of this PR.
This will also serve as a "contrib" folder, but to me calling a project
"contrib" makes it sound like it is unfinished or inferior quality. We expect
that these packages will be highly polished with a more intuitive API than the
Java ported code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
