SpinaCMS / Spina

Spina CMS
http://www.spinacms.com
Other
2.19k stars 405 forks source link

Is it possible to use CarrierWave instead of ActiveStorage? #404

Closed lukaVarga closed 2 years ago

lukaVarga commented 5 years ago

Pretty self explanatory - is it possible to use CarrierWave (like in Spina 0.x.x) instead of ActiveStorage for image handling via Spina?

Issues with AS:

So basically, we're bleeding performance, increasing load on our servers, generating unfriendly URLs for images and cannot use CDNs to improve performance.

With CarrierWave, everything was working out of the box. At the moment, I don't really see the benefit with using AS for a public blog use-case.

So, is there anything we can do or are we stuck with AS and monkey-patching (both Spina and Rails) in case we wish to use Spina 1.x.x?

Bramjetten commented 5 years ago

I get where you're coming from. In an effort to minimize our dependencies, we – perhaps prematurely – switched to ActiveStorage.

This alternative RepresentationsController is a nice bandaid for now:

# frozen_string_literal: true

module ActiveStorage
  class RepresentationsController < BaseController
    include ActiveStorage::SetBlob

    def show
      expires_in 1.year, public: true
      variant = @blob.representation(params[:variation_key]).processed
      send_data @blob.service.download(variant.key),
                type: @blob.content_type || DEFAULT_SEND_FILE_TYPE,
                disposition: 'inline'
    end
  end
end
lukaVarga commented 5 years ago

@Bramjetten thanks for the quick response!

This will only make the URL public, right? It won't help with additional DB queries, non-SEO friendly urls and other issues? I suppose I could monkey-patch it further and at least redirect it to use CDN, but that's still far from perfect. At this point, I am actually considering manually uploading images and manually adding links to templates, so instead of using Spina::Image, I'd be using Spina::Text... but that's sooo 🤢

Bramjetten commented 5 years ago

With this controller you can safely use a CDN and still use all goodies ActiveStorage provides. The only thing this doesn't tackle is your SEO-friendly URLs, but that's mostly related to filenames imo.

lukaVarga commented 5 years ago

Thanks, I'll try it out :)

lukaVarga commented 5 years ago

Ok, so this seems to be as good as it gets atm:

module ActiveStorage
  class RepresentationsController < BaseController
    include ActiveStorage::SetBlob

    def show
      url =
        Rails.cache.fetch("img_variants_#{@blob.key}_#{@blob.filename}_#{params[:variation_key]}", expires_in: 1.month) do
          expires_in 1.year, public: true
          @blob.representation(params[:variation_key]).processed.service_url(disposition: params[:disposition])
        end

      # custom method that does a simple string substitution to convert url from signed bucket to url from CDN and strips out the query params resulting from signed bucket url to leverage browser caching
      redirect_to ImagesHelper.cdn_url(bucket_url: @blob.service.bucket.url, image_url: url)
    end
  end
end

And

module ActiveStorage
  class BlobsController < BaseController
    include ActiveStorage::SetBlob

    def show
      url = @blob.service_url(disposition: params[:disposition], expires_in: 1.week)
      redirect_to ImagesHelper.cdn_url(bucket_url: @blob.service.bucket.url, image_url: url)
    end
  end
end

It'll still hit my server first, but at least it'll leverage CDN caching afterwards and it'll also be able to leverage user browser caching for images. I did consider making a helper that would already create a CDN link to assets in views, so the user wouldn't hit my server for loading images, but then I'd end up with non-SEO friendly image URLs (eg. https://foo.cloudfront.net/1z3aksc1cs3 whereas now I've at least got the SEO friendly version of /rails/active_storage/blobs/foobar/some-great-image-example.jpg). Looks like something like this will have to be done for FB OG images, though. It doesn't play too nicely with redirects or with relative paths..

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.