Skip to content

Commit c61bd2e

Browse files
author
Stanislav (Stas) Katkov
authored
Integrating collection renderer from ActionView (#43)
# Goal We should be able to effectively render collection of data and be able to cache it. ## Problem statement Previously, we have used following code to render collection: ```ruby pb.cache!(@account.offers_cache_key, expires_in: 12.hours) do pb.featured_offers @featured_offers, partial: "offer", as: :offer pb.normal_offers @normal_offers, partial: "offer", as: :offer end ``` But we need to introduce fragment caching to offers, since list cache has high miss rate. This is how implementing fragment caching in pbbuilder should look like: ```ruby pb.cache!(@account.offers_cache_key, expires_in: 12.hours) do pb.featured_offers partial: "offer", as: offer, collection: @featured_offers, cached: true pb.normal_offers partial: "offer", as: :offer, collection: @normal_offers, cached: true end ``` This syntax is heavily inspired by jbuilder. Here is list of examples from that gem that uses cached: and collection: attributes. ```ruby json.partial! partial: 'posts/post', collection: @posts, as: :post ``` ```ruby json.partial! 'posts/post', collection: @posts, as: :post ``` ```ruby json.partial! "post", collection: @posts, cached: true, as: :post ``` ```ruby json.array! @posts, partial: "post", as: :post, cached: true ``` ```ruby json.array! @posts, partial: "posts/post", as: :post, cached: true ``` ```ruby json.comments @post.comments, partial: "comments/comment", as: :comment, cached:true ``` ## Proposed solution In this PR, we're implementing more effective rendering of collection with a help of `ActiveView::CollectionRenderer`. Support for caching of partial is currently not in the scope, but this will help implement caching and caching digest later. Following syntax should be supported: ```ruby pb.friends partial: "racers/racer", as: :racer, collection: @Racers ``` ```ruby pb.friends "racers/racer", as: :racer, collection: @Racers ``` Previous syntax works as it used to before. These will remain unchanged and will not use collection renderer (at least for now) ```ruby pb.partial! @racer, racer: Racer.new(123, "Chris Harris", friends) ``` ```ruby pb.friends @friends, partial: "racers/racer", as: :racer ``` ## TODO - [x] Add documentation for this change - [ ] Check performance difference between Collection Renderer and our approach. (without cache for now) - [x] It's using PartialRenderer with rails 7 now - this seems like a bug, it should use CollectionRenderer. ## Prior work Some inspiration for this PR could be found here: * rails/jbuilder#501 * https://github.com/rails/rails/blob/main/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb#L20 * https://github.com/rails/rails/tree/main/actionview collection_renderer * https://github.com/rails/rails/blob/main/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb#L20
1 parent 1025f1e commit c61bd2e

15 files changed

+193
-67
lines changed

.github/workflows/test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
strategy:
1414
fail-fast: false
1515
matrix:
16-
ruby: ["2.7", "3.0", "3.1", "3.2"]
16+
ruby: ["3.0", "3.1", "3.2", "3.3"]
1717

1818
gemfile: [ "rails_6_1", "rails_7_0"]
1919
experimental: [false]

.ruby-version

-1
This file was deleted.

Appraisals

+3-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ appraise "rails-7-0" do
66
gem "rails", "~> 7.0.0"
77
end
88

9-
appraise "rails-head" do
10-
gem "rails", github: "rails/rails", branch: "main"
11-
end
9+
appraise "rails-7-1" do
10+
gem "rails", "~> 7.1.0"
11+
end

Gemfile

+2
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ gemspec
77

88
gem "rake"
99
gem "appraisal"
10+
11+
gem "ruby-lsp"

README.md

+30-5
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,26 @@
11
# Pbbuilder
22
PBBuilder generates [Protobuf](https://developers.google.com/protocol-buffers) Messages with a simple DSL similar to [JBuilder](https://rubygems.org/gems/jbuilder) gem.
33

4-
## Compatibility
5-
We don't aim to have 100% compatibility with jbuilder gem, but we closely follow jbuilder's API design.
4+
5+
At least Rails 6.1 is required.
6+
7+
## Compatibility with jBuilder
8+
We don't aim to have 100% compitability and coverage with jbuilder gem, but we closely follow jbuilder's API design to maintain familiarity.
69

710
| | Jbuilder | Pbbuilder |
811
|---|---|---|
912
| set! |||
1013
| cache! |||
1114
| cache_if! |||
1215
| cache_root! || |
16+
| collection cache || |
1317
| extract! |||
1418
| merge! |||
15-
| deep_format_keys! || |
1619
| child! || |
1720
| array! || |
18-
| ignore_nil! || |
21+
| .call || |
22+
23+
Due to protobuf message implementation, there is absolutely no need to implement support for `deep_format_keys!`, `key_format!`, `key_format`, `deep_format_keys`, `ignore_nil!`, `ignore_nil!`, `nil`. So those would never be added.
1924

2025
## Usage
2126
The main difference is that it can use introspection to figure out what kind of protobuf message it needs to create.
@@ -76,8 +81,26 @@ Here is way to use partials with collection while passing a variable to it
7681
pb.accounts @accounts, partial: "account", as: account
7782
```
7883

84+
## Collections (or Arrays)
85+
There are two different methods to render a collection. One that uses ActiveView::CollectionRenderer
86+
```ruby
87+
pb.friends partial: "racers/racer", as: :racer, collection: @racers
88+
```
89+
90+
```ruby
91+
pb.friends "racers/racer", as: :racer, collection: @racers
92+
```
93+
94+
And there are other ways, that don't use Collection Renderer (not very effective probably)
95+
```ruby
96+
pb.partial! @racer, racer: Racer.new(123, "Chris Harris", friends)
97+
```
98+
```ruby
99+
pb.friends @friends, partial: "racers/racer", as: :racer
100+
```
101+
79102
### Caching
80-
Fragment caching is supported, it uses Rails.cache and works like caching in HTML templates:
103+
it uses Rails.cache and works like caching in HTML templates:
81104

82105
```
83106
pb.cache! "cache-key", expires_in: 10.minutes do
@@ -93,6 +116,8 @@ pb.cache_if! !admin?, "cache-key", expires_in: 10.minutes do
93116
end
94117
```
95118

119+
Fragment caching support is currently in the works.
120+
96121
## Installation
97122
Add this line to your application's Gemfile:
98123

gemfiles/rails_6.gemfile

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# This file was generated by Appraisal
2+
3+
source "https://rubygems.org"
4+
5+
gem "rake"
6+
gem "appraisal"
7+
gem "rails", "~> 6.0"
8+
9+
gemspec path: "../"

gemfiles/rails_6_1.gemfile

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ source "https://rubygems.org"
44

55
gem "rake"
66
gem "appraisal"
7+
gem "ruby-lsp"
78
gem "rails", "~> 6.1.0"
89

910
gemspec path: "../"

gemfiles/rails_7.gemfile

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# This file was generated by Appraisal
2+
3+
source "https://rubygems.org"
4+
5+
gem "rake"
6+
gem "appraisal"
7+
gem "rails", "~> 7.0"
8+
9+
gemspec path: "../"

gemfiles/rails_7_0.gemfile

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ source "https://rubygems.org"
44

55
gem "rake"
66
gem "appraisal"
7+
gem "ruby-lsp"
78
gem "rails", "~> 7.0.0"
89

910
gemspec path: "../"

gemfiles/rails_7_1.gemfile

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# This file was generated by Appraisal
2+
3+
source "https://rubygems.org"
4+
5+
gem "rake"
6+
gem "appraisal"
7+
gem "ruby-lsp"
8+
gem "rails", "~> 7.1.0"
9+
10+
gemspec path: "../"

lib/pbbuilder.rb

+37-53
Original file line numberDiff line numberDiff line change
@@ -40,42 +40,49 @@ def method_missing(...)
4040
set!(...)
4141
end
4242

43+
def attributes!
44+
@message.to_h
45+
end
46+
4347
def respond_to_missing?(field)
44-
!!@message.class.descriptor.lookup(field.to_s)
48+
!!_descriptor_for_field(field)
4549
end
4650

4751
def set!(field, *args, &block)
4852
name = field.to_s
49-
descriptor = @message.class.descriptor.lookup(name)
53+
descriptor = _descriptor_for_field(name)
5054
::Kernel.raise ::ArgumentError, "Unknown field #{name}" if descriptor.nil?
5155

52-
if block
56+
if ::Kernel.block_given?
5357
::Kernel.raise ::ArgumentError, "can't pass block to non-message field" unless descriptor.type == :message
5458

5559
if descriptor.label == :repeated
5660
# pb.field @array { |element| pb.name element.name }
5761
::Kernel.raise ::ArgumentError, "wrong number of arguments #{args.length} (expected 1)" unless args.length == 1
5862
collection = args.first
5963
_append_repeated(name, descriptor, collection, &block)
60-
return
64+
else
65+
# pb.field { pb.name "hello" }
66+
::Kernel.raise ::ArgumentError, "wrong number of arguments (expected 0)" unless args.empty?
67+
message = (@message[name] ||= _new_message_from_descriptor(descriptor))
68+
_scope(message, &block)
6169
end
62-
63-
::Kernel.raise ::ArgumentError, "wrong number of arguments (expected 0)" unless args.empty?
64-
# pb.field { pb.name "hello" }
65-
message = (@message[name] ||= _new_message_from_descriptor(descriptor))
66-
_scope(message, &block)
6770
elsif args.length == 1
6871
arg = args.first
6972
if descriptor.label == :repeated
7073
if arg.respond_to?(:to_hash)
7174
# pb.fields {"one" => "two"}
72-
arg.to_hash.each do |k, v|
73-
@message[name][k] = v
74-
end
75-
elsif arg.respond_to?(:to_ary)
75+
arg.to_hash.each { |k, v| @message[name][k] = v }
76+
elsif arg.respond_to?(:to_ary) && !descriptor.type.eql?(:message)
7677
# pb.fields ["one", "two"]
7778
# Using concat so it behaves the same as _append_repeated
79+
7880
@message[name].concat arg.to_ary
81+
elsif arg.respond_to?(:to_ary) && descriptor.type.eql?(:message)
82+
# pb.friends [Person.new(name: "Johnny Test"), Person.new(name: "Max Verstappen")]
83+
# Concat another Protobuf message into parent Protobuf message (not ary of strings)
84+
85+
args.flatten.each {|obj| @message[name].push descriptor.subtype.msgclass.new(obj)}
7986
else
8087
# pb.fields "one"
8188
@message[name].push arg
@@ -103,10 +110,7 @@ def set!(field, *args, &block)
103110
end
104111

105112
def extract!(element, *args)
106-
args.each do |arg|
107-
value = element.send(arg)
108-
@message[arg.to_s] = value
109-
end
113+
args.each { |arg| @message[arg.to_s] = element.send(arg) }
110114
end
111115

112116
# Merges object into a protobuf message, mainly used for caching.
@@ -115,11 +119,11 @@ def extract!(element, *args)
115119
def merge!(object)
116120
::Kernel.raise Pbbuilder::MergeError.build(target!, object) unless object.class == ::Hash
117121

118-
object.each_key do |key|
119-
next if object[key].respond_to?(:empty?) && object[key].empty?
122+
object.each do |key, value|
123+
next if value.respond_to?(:empty?) && value.empty?
120124

121-
descriptor = @message.class.descriptor.lookup(key.to_s)
122-
::Kernel.raise ::ArgumentError, "Unknown field #{name}" if descriptor.nil?
125+
descriptor = _descriptor_for_field(key)
126+
::Kernel.raise ::ArgumentError, "Unknown field #{key}" if descriptor.nil?
123127

124128
if descriptor.label == :repeated
125129
# optional empty fields don't show up in @message object,
@@ -128,50 +132,26 @@ def merge!(object)
128132
@message[key.to_s] = _new_message_from_descriptor(descriptor)
129133
end
130134

131-
if object[key].respond_to?(:to_hash)
132-
object[key].to_hash.each {|k, v| @message[key.to_s][k] = v}
133-
elsif object[key].respond_to?(:to_ary)
134-
elements = object[key].map do |obj|
135+
if value.respond_to?(:to_hash)
136+
value.to_hash.each {|k, v| @message[key.to_s][k] = v}
137+
elsif value.respond_to?(:to_ary)
138+
elements = value.map do |obj|
135139
descriptor.subtype ? descriptor.subtype.msgclass.new(obj) : obj
136140
end
137141

138142
@message[key.to_s].replace(elements)
139143
end
140144
else
141-
if object[key].class == ::String
145+
if descriptor.type == :message
146+
@message[key.to_s] = descriptor.subtype.msgclass.new(value)
147+
else
142148
# pb.fields {"one" => "two"}
143-
@message[key.to_s] = object[key]
144-
elsif object[key].class == ::TrueClass || object[key].class == ::FalseClass
145149
# pb.boolean true || false
146-
@message[key.to_s] = object[key]
147-
elsif object[key].class == ::Array
148150
# pb.field_name do
149151
# pb.tags ["ok", "cool"]
150152
# end
151153

152-
@message[key.to_s] = object[key]
153-
elsif object[key].class == ::Hash
154-
if @message[key.to_s].nil?
155-
@message[key.to_s] = _new_message_from_descriptor(descriptor)
156-
end
157-
158-
object[key].each do |k, v|
159-
if object[key][k].respond_to?(:to_hash)
160-
if @message[key.to_s][k.to_s].nil?
161-
descriptor = @message[key.to_s].class.descriptor.lookup(k.to_s)
162-
@message[key.to_s][k.to_s] = _new_message_from_descriptor(descriptor)
163-
end
164-
165-
_scope(@message[key.to_s][k.to_s]) { self.merge!(object[key][k]) }
166-
elsif object[key][k].respond_to?(:to_ary)
167-
@message[key.to_s][k.to_s].replace object[key][k]
168-
else
169-
# Throws an error, if we try to merge nil object into empty value.
170-
next if object[key][k].nil? && @message[key.to_s][k.to_s].nil?
171-
172-
@message[key.to_s][k.to_s] = object[key][k]
173-
end
174-
end
154+
@message[key.to_s] = value
175155
end
176156
end
177157
end
@@ -184,6 +164,10 @@ def target!
184164

185165
private
186166

167+
def _descriptor_for_field(field)
168+
@message.class.descriptor.lookup(field.to_s)
169+
end
170+
187171
# Appends protobuf message with existing @message object
188172
#
189173
# @param name string

lib/pbbuilder/collection_renderer.rb

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# frozen_string_literal: true
2+
3+
require 'action_view/renderer/collection_renderer'
4+
5+
class Pbbuilder
6+
class CollectionRenderer < ::ActionView::CollectionRenderer # :nodoc:
7+
def initialize(lookup_context, options, &scope)
8+
super(lookup_context, options)
9+
@scope = scope
10+
end
11+
12+
private
13+
14+
def build_rendered_template(content, template, layout = nil)
15+
super(content || pb.attributes!, template)
16+
end
17+
18+
def build_rendered_collection(templates, _spacer)
19+
pb.set!(field, templates.map(&:body))
20+
end
21+
22+
def pb
23+
@options[:locals].fetch(:pb)
24+
end
25+
26+
def field
27+
@options[:locals].fetch(:field).to_s
28+
end
29+
end
30+
end

lib/pbbuilder/pbbuilder.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
# frozen_string_literal: true
22

33
Pbbuilder = Class.new(begin
4-
require 'active_support/proxy_object'
5-
ActiveSupport::ProxyObject
4+
require 'active_support/proxy_object'
5+
ActiveSupport::ProxyObject
66
rescue LoadError
7-
require 'active_support/basic_object'
8-
ActiveSupport::BasicObject
7+
require 'active_support/basic_object'
8+
ActiveSupport::BasicObject
99
end)

lib/pbbuilder/template.rb

+26
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# frozen_string_literal: true
22

3+
require 'pbbuilder/collection_renderer'
4+
35
# PbbuilderTemplate is an extension of Pbbuilder to be used as a Rails template
46
# It adds support for partials.
57
class PbbuilderTemplate < Pbbuilder
@@ -37,6 +39,30 @@ def set!(field, *args, **kwargs, &block)
3739
super(field, *args) do |element|
3840
_set_inline_partial(element, kwargs)
3941
end
42+
elsif kwargs.has_key?(:collection) && kwargs.has_key?(:as)
43+
# pb.friends partial: "racers/racer", as: :racer, collection: [Racer.new(1, "Johnny Test", []), Racer.new(2, "Max Verstappen", [])]
44+
# collection renderer
45+
options = kwargs.deep_dup
46+
47+
options.reverse_merge! locals: options.except(:partial, :as, :collection, :cached)
48+
options.reverse_merge! ::PbbuilderTemplate.template_lookup_options
49+
50+
collection = options[:collection] || []
51+
partial = options[:partial]
52+
options[:locals].merge!(pb: self)
53+
options[:locals].merge!(field: field)
54+
55+
if options.has_key?(:layout)
56+
raise ::NotImplementedError, "The `:layout' option is not supported in collection rendering."
57+
end
58+
59+
if options.has_key?(:spacer_template)
60+
raise ::NotImplementedError, "The `:spacer_template' option is not supported in collection rendering."
61+
end
62+
63+
CollectionRenderer
64+
.new(@context.lookup_context, options) { |&block| _scope(message[field.to_s],&block) }
65+
.render_collection_with_partial(collection, partial, @context, nil)
4066
else
4167
# pb.best_friend partial: "person", person: @best_friend
4268
# Call set! as a submessage, passing in the kwargs as partial options

0 commit comments

Comments
 (0)