Github user AndyPook commented on the issue:

    https://github.com/apache/lucenenet/pull/209
  
    Making ```Perform``` async could be done like... 
    
    ```csharp
            public async Task PerformAsync(IReplicationRequest request, 
IReplicationResponse response)
            {
                ...
                try
                {
                    switch (action)
                    {
                        case ReplicationAction.OBTAIN:
                            string sessionId = ExtractRequestParam(request, 
REPLICATE_SESSION_ID_PARAM);
                            string fileName = ExtractRequestParam(request, 
REPLICATE_FILENAME_PARAM);
                            string source = ExtractRequestParam(request, 
REPLICATE_SOURCE_PARAM);
                            using (Stream stream = 
replicator.ObtainFile(sessionId, source, fileName))
                                await stream.CopyToAsync(response.Body);
                            break;
    
                        case ReplicationAction.RELEASE:
                            replicator.Release(ExtractRequestParam(request, 
REPLICATE_SESSION_ID_PARAM));
                            break;
    
                        case ReplicationAction.UPDATE:
                            string currentVersion = 
request.QueryParam(REPLICATE_VERSION_PARAM);
                            SessionToken token = 
replicator.CheckForUpdate(currentVersion);
                            if (token == null)
                            {
                                await response.Body.WriteAsync(new byte[] { 0 
}, 0, 1); // marker for null token
                            }
                            else
                            {
                                await response.Body.WriteAsync(new byte[] { 1 
}, 0, 1);
                                token.Serialize(new 
DataOutputStream(response.Body));
                            }
                            break;
                        default:
                            throw new ArgumentOutOfRangeException();
                    }
                }
                ...
            }
    ```
    Note ```CopyToAsync``` and ```WriteAsync```. It's a shame that 
DataOutputStream isn't async. A little out of scope for this though :)
    
    On the trivia front. The ReplicatorBuilder fluent methods can just 
```return this``` rather than newing up another builder each time.
    
    Shouldn't the template building part be
    ```
    string template = (string.IsNullOrWhiteSpace(urlPrefix) ? "" : urlPrefix) +
                    "/{context}/{shard}/{action}";
    ```
    Otherwise the template would not start with "/" if urlPrefix was missing 
and it would throw?
    
    Though I would suggest that urlPrefix should be mandatory. Otherwise it 
will steal all the requests from whatever comes next in the middleware pipeline.
    
    Q: Why do you say it's "horrible to block the default route"?
    This seems entirely natural to me :) Even back in asp you could add 
handlers that would intercept certain paths/extensions. It doesn't seem any 
different to Swagger intercepting "/swagger". Or OAuth intercepting it's 
control flow.
    
    Lastly, an observation... My intent with using the template was that the 
Perform method would be refactored to accept the parts from the route. If you 
want to leave the Perform method as is (closer to java implementation I'm 
guessing) which parses the path and qs then the ```MapWhen``` approach might be 
better/simpler...
    
    ```csharp
                public static void UseLuceneReplicator3(this 
IApplicationBuilder app, string urlPrefix, object indexService)
                {
                        if (string.IsNullOrEmpty(urlPrefix) || 
!urlPrefix.StartsWith("/"))
                                throw new ArgumentException("urlPrefix MUST be 
provided");
    
                        var replicatorAccessor = 
app.ApplicationServices.GetService<IReplicatorAccessor>();
                        var replicationService = new 
ReplicationService(replicatorAccessor.Replicators);
    
                        app.MapWhen(context => 
context.Request.Path.StartsWithSegments(urlPrefix), appBuilder =>
                        {
                                appBuilder.Run(async context =>
                                {
                                        await replicationService.PerformAsync(
                                                new 
AspNetCoreReplicationRequest(context.Request), 
                                                new 
AspNetCoreReplicationResponse(context.Response));
                                });
                        });
                }
    ```
    
    As the accessor and service are singletons, they only need resolving once 
rather than every request.
    
    Lastly, the template we've been looking at has a {context] segement, 
however the Perform method only parses 'shard' and 'action'. Is that 
significant?
    
    Just stream of thought and observations :)


---
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