andrea-magni / MARS

MARS-Curiosity Delphi REST Library
Mozilla Public License 2.0
362 stars 113 forks source link

Server compression #49

Closed ghost closed 2 years ago

ghost commented 5 years ago

Hello, it would be great to have compression for server responces.

andrea-magni commented 5 years ago

I usually enable zip compression (as well as SSL encryption) through a reverse proxy (Apache or nGinx). I tend to expose to the web this proxy (Apache most of the time) and keep my MARS server behind it for security reasons and to address SSL configuration (far easier to handle there or just because the customer already has something setup in this area).

But I agree having something built in can be handy and it should not be hard to implement. Will let you know

andrea-magni commented 5 years ago

I had a quick look and it seems simple to support 'deflate' compression method for server's responses: just add the compression code (ZipStream function from MARS.Core.Utils comes handy here) right after MARS Invocation and substitute the output stream. You can implement whatever behavior you may like (i.e. compress every JSON response that is above 1024 bytes in length, like in the following example) to tune performances or your strategies (use a custom attribute to compress only some resources or methods and so on...).

    // to execute something after each activation
    TMARSActivation.RegisterAfterInvoke(
      procedure (const AActivation: IMARSActivation)
      begin
        var LResp := AActivation.Response;
        if (LResp.ContentType = TMediaType.APPLICATION_JSON)
          and (LResp.ContentLength >= 1024)
        then
        begin
          var LCompressed := TMemoryStream.Create();
          try
            ZipStream(LResp.ContentStream, LCompressed);
            LResp.ContentStream.Free;
            LResp.ContentStream := LCompressed;
            LResp.ContentEncoding := 'deflate';
          except
            LCompressed.Free;
            raise;
          end;
        end;
      end
    );

The other way around seems a bit different as you can't manipulate Request's content before MARS invocation (AFAIK, just had a quick look) but you can actually improve the MessageBodyReaders in order to handle decompression ahead of deserialization. Is it a common scenario to have compressed data from client to server? (as I said, I tend to let encryption, compression and load balancing to a proxy in front of my application servers so I am not really an expert of these kind of details).

ghost commented 5 years ago

I've already made response compression inside TMARShttpServerIndy.DoCommandGet procedure, but your approach is much cleaner I think. But there is a bug in TNetHTTPClient at least in 10.2.3. On iOS it works good with compressed responses, but on Android I had to decompress data inside HttpClientRequestCompleted event. I do not need to compress data from client to server, but decompression on the server side can be done in TMARShttpServerIndy.BeforeCommandGet event. By the way, why is this event exists at all, as it is possible just to change visibility of OnCommandGet event of ancestor class?

andrea-magni commented 5 years ago

I'll try to delve into the TNetHTTPClient thing as my client library has a TMARSClient implementation that is based on that library too (TMARSNetClient). Thanks for pointing this out, is there any issue opened on quality.embarcadero.com already about it?

For the TMARShttpServerIndy thing, please keep in mind MARS can be deployed also without Indy as http server (i.e. as Apache module or IIS ISAPI DLL and more to come). So using BeforeInvoke/AfterInvoke would be more convenient and portable (not tied to Indy implementation). How are you doing with MARS, if I can ask, some feedback is always precious and I am looking for it. :-)

andrea-magni commented 5 years ago

Thinking twice, I was completely wrong! Compression/Decompression is a http-level functionality so should be out of MARS Activation level. In other words, it is better to implement compression/decompression at Indy level (TMARShttpServerIndy) as you did and not as Before|AfterInvoke as I did. Just thinking about deploying to Apache or IIS leads to the fact that at that point one may prefer compression/decompression from the hosting webserver thus the second compression trigger (at Before|AfterInvoke point) should be disabled.

I think I will implement this in TMARShttpServerIndy and push it to the repo soon. Thanks @dustypup for hints and driving my attention in the right direction!

ghost commented 5 years ago

Yes, I'am talking exactly about TNetHTTPClient in your TMARSClient component. No, there are no issues about this bug, but it could have been resolved in Rio. Need to test, but I have no time for moving to Rio right now.

I'am planning to use ISAPI for production, but now I use Indy Server as it is easier to debug.

As for "doing with MARS". Thanks for your work. I'am developing mobile client for already running CRM, so I need a REST server. I've tried DataSnap, but it is too "huge" and "clumsy" =) For now MARS suits good for me. It is lightweight, easy to understand and manage the whole REST thing. Of course, it would be great to have better documentation and more meaningfull demos...

ghost commented 5 years ago

Yes, I agree that compression should be done at the server layer. Activation layer is cleaner, as I've mentioned before, but only if you dont want to touch server sources. As for implementation... It is not so simple if you want to make it RFC compartible. =) Anyway I can help implementing this feature, if you need help of course =)

andrea-magni commented 5 years ago

Thanks for the feedback and yes MARS lacks documentation (I am trying to get the library some popularity through conferences and public events but I probably need a more structured approach to build some documentation for everybody who wants to learn it). If you have suggestions for useful demos please open an issue for each idea and I'll try to cover them ASAP. And your help for compression implementation at Indy level would be very welcomed! Provide a pull request and I will be happy to merge it.

Thanks!

andrea-magni commented 5 years ago

Hi @dustypup, any news about this? Do you have an idea about when would you be able to provide your contribution, if you still can/want?

Sincerely, Andrea

ghost commented 5 years ago

Hi, @andrea-magni. I was on vacation. Will have time complete on the next week.

andrea-magni commented 5 years ago

@dustypup No worries and hope you had good time while on vacation :-)

bogdanpolak commented 4 years ago

@dustypup Quite a long vacations :-) I think we should close this item