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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to