lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From AndyPook <...@git.apache.org>
Subject [GitHub] lucenenet issue #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Date Tue, 08 Aug 2017 09:15:50 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message