(Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)
We are using exception_notification gem. It is great because it will send us a direct email when there is an error on the platform. This means we are not looking at the logs. We get the notification directly on the email.
But some of the crawlers (like all of them) cause errors, mainly in the cross-origin domain. The JavaScript we are delivering is not allowed for their domain and this generates an error. One way to stop these errors is to ignore errors caused crawlers in the exception_notifcation configuration:
You don’t want forgery protection on some API controllers. If the API controllers extend the ActionController::Base the forgery protection could be disabled in the following way. Here is an example for a controller accepting requests from Amazon AWS SNS.
class SnsController < ActionController::Base
# Disable it for this controller.
# If there is no session it is just null session
protect_from_forgery with: :null_session
http_basic_authenticate_with :name => ENV["SNS_USERNAME"], :password => ENV["SNS_PASSWORD"]
...
end
An even better approach would be not extending from ActionController::Base, but from ActionController::API. But then we would have to include the modules for HttpAuthentication which is a topic for another article.
The issue occurs because of a new security patch with with Rails tracked with CVE-2021-22885.
You can no longer call polymorphic_path or other dynamic path helpers with strings, because if this strings are provided by the user this could result in unwanted router helper calls.
# previous call
polymorphic_path([article, "authors"])
# should now be
polymorphic_path([article, "authors".to_sym])
# or better
polymorphic_path([article, :authors])
Migration effort
A perspective on how serious it is to upgrade – tl;dr – it is not – about half an hour.
All the calls in our platform for polymorphic_path
$ git grep "polymorphic_path" | wc -l
321
All the file that have calls to polymorphic_path
$ git grep -l "polymorphic_path" | wc -l
143
Numbers of files that I’ve changed – 13 Time it took me to review all of them -16 minutes, from 18:24 to 18:40
Now I am waiting for specs to pass.
I guess it is not a big change and could be migrate in half an hour. Most of our calls were using symbols already and only about 15 calls from 321 were with strings. These are 4% of all the calls.
(Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)
One thing that is good to know:
If you render plain text from a web framework like Rails you will not have plain text in the browser. You will still have HTML in the browser.
This bite us with one of the RSpec scenarios that we were running and I thought I should share with the team and the whole community.
IRL
The real life scenario is the following.
In the Rails controller we do
class DigestMessagesController < ApplicationController
def show
render plain: "Text"
end
end
What we would expect to see in the browser is a simple “Text” text. If fact this is exactly what is returned by the server – you can see in the “Response” tab.
you can see that the response from the server is just “Text”
But if you now Inspect the content of the page in the browser you will see that there is HTML inside.
This HTML was not returned by the framework. This HTML was added by the browser. This is not good because an RSpec system scenario with Capybara will pass with rack_test driver, but will fail with a Selenium driver. rack_test driver will not add this HTML while all the major browsers will add it.
scenario 'for text' do
visit "/digest_messages/#{digest_message.id}?preview=plain"
expect(page.body.to_s).to include("Text")
# This expect here will fail for Selenium and will pass for rack_test driver.
expect(page.body.to_s).not_to include("html")
end
I hope this is helpful and could save you a couple of minutes debugging.
(Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)
This is an article of a production incidents with Rails/Stimulus/Rails-ujs/DOM and jQuery.
We made mistake that made us lose 31 registrations on the FLLCasts and a few more on the BuildIn3D platform. The stack involved includes Ruby on Rails with Rails-ujs, Stimulus, DOM and jQuery. I will enter into some details about how it occurred, why and how we resolved it in under an hour. I am writing this article mainly to share with my our team, but I am sure it could be useful for other teams.
FLLCasts registration form as of April 2021
The incident – people could not Sign Up on the platforms
On the Sign up page there is a captach. Years ago we found our that the captcha helps us reduce invalid registartion. There is also one more thing on this platform – usernames. As the FLLCasts platforms is used by students and academies, one of the very useful features on the platform is the automatically generated username. Users type their names in the “Full name” field and a username is automatically generated for them. In this way there is a unique username for users because email is not unique – there could be many users with the same email.
To generate a new username we send a request to the server and refresh form. We use Stimulus and rails-ujs for this.
The is an “change->registrations#refreshForm” for the input field for the name. When the full name is changed the form is refreshed and a unique username is returned from the server.
The incident was that when the form was refresh the captcha got lost and we were not showing it after that. This means that on the server we were checking the captcha, but there was not captcha on the client.
The ticket
A wild ticket appears from a user.
Hi,
We wanted to enroll our Kid for Personal B programme and we were trying to Sign Up to pay online , but we are unable to sign up using our details.
We get an error message though we tried a few times.
Please help to sort this out.
Thanks
Regards
This is a really wild ticket. The user wants to buy and they can not sign up. 1,2,3 go.
The problem
We’ve made the following commit in the registration form
--- a/app/views/devise/registrations/new.js.erb
+++ b/app/views/devise/registrations/new.js.erb
@@ -1 +1 @@
-$("#<%= @form_id %>").replaceWith("<%= escape_javascript(render 'form') %>")
\ No newline at end of file
+document.getElementById("<%=form_id%>").outerHTML= '<%= escape_javascript(render "#{partial_name}") %>';
\ No newline at end of file
We are in the process of removing jQuery from the code. I saw this jQueyr call and decided to change it to a simple DOM call. It should be the same, right? No…
jQuery executes scrips while document.outerHTML does not
I knew this. But I did not consider it in this commit. What happens is that we receive the new form from the server and replace the form on the page with the new form received from the server that contains the generate username.
But in this form (and this form only on the whole platform) there is a recaptach. This recaptcha is a JavaScript and when dynamically changing the DOM the JavaScript must be executed. Well, jQuery automatically executed this for us. document.outerHTML does not.
Developing and RSpec system spec.
First I deployed the fix to production. It took about 5 minutes.
After that I developed the spec. This is how the spec looks like:
scenario "shows recaptach when dynamically reloading the form", js: true do
with_recaptcha_enabled do
visit_sign_up
expect(page).to have_recaptcha
name_field.set unique_name
email_field.set "#{SecureRandom.hex(16)}@example.com"
# This means we have reloaded with js because we've set the email field
# and this has change the focus and there was a fire event for the name_field.
#
# We are now waiting for the username to appear on the screen
expect(page).to have_username_filled
expect(page).to have_recaptcha
end
end
We expect that there is a recaptcha before and after refreshing the form.
What surprised us
Looking at the data in the time frame this commit was on production – we’ve received 9 registrations on the platform. In the same previous period we’ve receive 40 registrations.
This means that we’ve lost 31 registrations on the platform.
What surprised us is that 9 people managed to register. The only way you could register on the platform during this period is to first enter your username. If you enter the username on your own we do not refresh the form as we don’t have to generate a username for you. This means that about 25% of the people start with their username and about 75% of the users at FLLCasts start the registration with their name or email.
Good to know. Good to know.
Conclusion
Looking at the issue we could have prevented it in a number of ways. In a test environment it is difficult to have a recaptcha because there is no way to test that the recaptcha works. After all that is the whole purpose of a recaptcha – to prevent bots and a Selenium driver is exactly a bot. But it turned out we’ve missed it as a scenario in the specs. It is always a missing spec.
Have you used gems that provide class methods for Ruby on Rails controllers that extend these controllers?
Like the ones below:
class TutorialsController < ApplicationController
# This is from cancancan
# It allows to loading and authorizing
load_and_authorize_resource
...
# This is from 'has_scope'.
has_scope :pref_order, only: :index, default: "suggested"
end
Today’s article is about how to implement such a method for our own concern.
My specific case is duplication. I would like to develop a DuplicateResourceConcern that extends the controller and allows users to duplicate a record when they create a new record. This is something we’ve been using at FLLCasts and BuildIn3D for many years, but today I revised the implementation, improved it and decided to share the knowledge in a short article.
The goal is to be able to do:
class TutorialsController < ApplicationController
include DuplicateResourceConcern
enable_duplicate
...
end
With this logic when the user:
visits /tutorials/new?resource_to_dup=123
The title field in the form for new tutorial is pre-filled the title of Tutorial 123.
Declare DuplicateResourceConcern and enable_duplicate
First we declare DuplicateResourceConcern
module DuplicateResourceConcern
extend ActiveSupport::Concern
included do
before_action :dup, only: [:new]
end
private
def dup
# call the duplication
end
end
It is a concern with one method that is ‘dup’
We use the concern in the TutorialsController
class TutorialsController < ApplicationController
include DuplicateResourceConcern
def new
end
end
The method ‘dup’ is a private method of the controller. It is called before the :new action. If a param called ‘resource_to_dup=X’ is passed we would like to duplicate the title of Tutorial X.
Simple Tutorial duplication
module DuplicateResourceConcern
extend ActiveSupport::Concern
included do
before_action :dup, only: [:new]
end
private
def dup
original_tutorial = Tutorial.find_by_id(params.permit(:resource_to_dup)[:resource_to_dup])
@tutorial = Tutorial.new(title: original_tutorial.title)
end
end
This duplication will work for the Tutorials controller. But it will not work for ArticlesController, because we are doing “Tutorial.find” and we are assigning a @tutorial variable.
Reusing the DuplicateResourceConcern in a different controller
To reuse the DuplicateResoureConcern we must call Article.find_by_id when we are in an ArticlesController and Tutorial.find_by_id when we are in a TutorialsController.
Here is how we could do this using ‘controller_name’, ‘.classify’ and ‘.constantize’. We can get the name of the records from the controller name.
module DuplicateResourceConcern
extend ActiveSupport::Concern
included do
before_action :dup, only: [:new]
end
private
def dup
# This will return Tutorial for TutorialsController
# and will return Article for ArticlesController
clazz = controller_name.classify.constantize
# If the controller is called 'tutorials' the instance name will be 'tutorial'
instance_name = controller_name.singularize
original_instance = clazz.find_by_id(params.permit(:resource_to_dup)[:resource_to_dup])
duplicated_instance = clazz.new(title: original_instance.title)
# After this call we will be able to access @tutorial in the TutorialsController and @article in the ArticlesController
instance_variable_set("@#{instance_name}", duplicated_instance)
end
end
In this way the DuplicateResourceConcern is not dependent on the type of the object. It could work for Articles and for Tutorials. It is still dependent on the property ‘title’, but I will leave this for now.
How to configure the duplication
The name of the parameter in the url should be ‘resource_to_dup’. What if we want to modify this name on the controller level. For example to have a parameter ‘resource_to_dup‘ for TutorialsController and ‘resource_to_copy‘ for ArticlesController.
What we want to do is pass an argument with the name of the parameter, but we would have to do a little more work.
class TutorialsController < ApplicationController
include DuplicateResourceConcern
enable_duplicate param_name: "my_new_param_name"
def new
end
end
We would use the class_method for the concerns.
module DuplicateResourceConcern
extend ActiveSupport::Concern
included do
before_action :dup, only: [:new]
end
def self.included(base)
base.class_eval do
extend ClassMethods
# Declare a class-level attribute whose value is inheritable by subclasses. Subclasses can change their own value and it will not impact parent class.
# In this way we can extend the class and still have separate configurations
class_attribute :duplicate_resource_configuration, instance_writer: false
self.duplicate_resource_configuration = {
options: {
param_name: "resource_to_dup",
}
}
end
end
class_methods do
# The enable_duplicate method will be called on a class level.
def enable_duplicate(*args) do
options = args.extract_options!
options.symbolize_keys!
# We check that there are only parameters with this names.
options.assert_valid_keys(:param_name)
# Merge the default options with the options passed in the controller
self.duplicate_resource_configuration[:options].merge!(options)
# We mark that we've enabled the duplication
self.duplicate_resource_configuration[:enabled] = true
end
end
private
def dup
# This will return Tutorial for TutorialsController
# and will return Article for ArticlesController
clazz = controller_name.classify.constantize
# If the controller is called 'tutorials' the instance name will be 'tutorial'
instance_name = controller_name.singularize
# Param name. We are no longer dependent on
# the param name being resource_to_dup. It could
# be a different name.
the_param_name = self.duplicate_resource_configuration[:options][:param_name]
original_instance = clazz.find_by_id(params.permit(the_param_name)[the_param_name])
duplicated_instance = clazz.new(title: original_instance.title)
# After this call we will be able to access @tutorial in the TutorialsController and @article in the ArticlesController
instance_variable_set("@#{instance_name}", duplicated_instance)
end
end
Extensibility
This is the basic structure. We can now add more options if we want to and more parameters to the configurations.
What is the system RSpec for duplication.
It is simple.
scenario "/tutorials/new?resource_to_dup can duplicate a resource" do
tutorial.update(title: SecureRandom.hex(10))
visit "/tutorials/new?resource_to_dup=#{tutorial.id}"
expect(page).to have_text "Duplicate from #{tutorial.title}"
click_on "Create Tutorial"
expect(page).to have_text "Tutorial was successfully created."
tutorials = Tutorial.where(title: tutorial.title)
expect(tutorials.count).to eq 2
end
(Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)
Today’s article is about a performance issue that we had in one of our Rails platforms and how we resolved it. I enter into details of the specific root cause and how New relic was helpful in identifying the issue. This article is mainly targeted for our team, but I hope it would be useful for the community to see a practical example of cancancan and N+1.
The root cause
Every time we show a lesson on the FLLCasts platform we show the tutorials and materials of that lesson. But if the user does not have access to a tutorial, because it is not published or it requires a teachers subscription we do not show it in the lesson. We filter it from the lesson.
We are using cancancan and we’ve added code for checking if every tutorial in the course could be accessed – whether you have the right subscription. This was not a problem on its own. The major problem was that we were showing a navigation of the course on the right that contains links to all the lessons with all the tutorials:
Show of links to all the tutorials in the whole course in the navigation
To show the navigation we looped through all the tutorials in the whole course and checked if the user has access to them. It was an N+1. This seems to be working when a course contains < 100 tutorials. But once the courses grew and contained more tutorials the logic got slow.
People started noticing. We had to increase the number of Heroku dynos from time to time. Especially when a lot of students join and use the platform.
There were two problems
We don’t need to show all the tutorials and building instructions in the navigation. We just need to show them for the current lesson.
We need to make a singe query to the DB that would get only the tutorials in that lesson and only the tutorials the user has access to.
What was the performance impact
During load when a lot of students were using the platform the navigation/_course_section partial was taking 41% of the show lesson request average of 5 seconds. There were about 100 find queries for a single lesson.
Table with how expensive it was to show the navigation for large courses
It was taking about 10-15 seconds for the server to respond and sometimes requests were failing.
Responses were slow. > 15 second sometimes.
On the chart below we see how the moment people start opening courses the time for showing lessons (course_sections) starts growing.
Sharp increase in time the moment we make requests
Practically the platform was unusable in this case. Everything stopped.
Solutions
Change the navigation to include only the current lesson
We decided that we don’t need to show all the tutorials from the whole course in the navigation, only the tutorials for the current lessons. This reduced the load because instead of returning 140 tutorials we were returning 20.
Improve the call to cancancan
One thing that we’ve missed is a call to cancancan that is can? :show for each record. It has slipped in the years and we haven’t noticed.
records.each do |record|
if can? :show, record
...
end
end
I refactored it to use cancancan accessible_by method.
and added two new :index rules in the ContentRefs ability. The benefit is that in this way cancancan makes a single query to the db.
module Abilities
class ContentRefsAbility
include CanCan::Ability
def initialize user
can :index, ContentRef, { archived: [nil,false], hidden: false}
can :index, ContentRef, { archived: true, hidden: [true,false], course_section: { authors: { user_id: user.id }}}
...
can :read, ContentRef, { archived: [nil,false], hidden: true ,course_section: {course: {required_access_for_hidden_refs: {weight: 0..user_max_plan_weight}}}}
...
end
end
end
The third rule is the deal breaker. With it we check if the user has a subscription plan that would allow them to access the content ref.
With these three rules we allow everybody to see non archive and non hidden content_refs, we also allow authors of the course_section to see archived and hidden content_refs where they are the author of the course_section and we allow allow everybody to see hidden content_refs if they have the proper subscription plan.
Increase in throughout put does not increase the required time
It does not matter whether we have 1 or 40 rpm for this controller. The time stays the same. Next we can focus on how to make it even faster, but this will do until the platform grows x10
(Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)
We can all agree that dead code should be removed. What we sometimes fail to see is how much it could cost to leave dead code with our product. Today’s article is about an example of a “dead code” and how it hit us back 1.5 years after we stopped using it. It took us 3 hours to debug and find the root cause. The conclusion is that we should have done ‘git rm’ an year ago. We did not and the cost was 3 hours.
This is a real life example article from our code base and it is design to share experience with our team, but I hope the whole community could benefit.
What happened
1. Jenkins build failed
First time we saw it we identified that it is not something serious and it kept failing for a few days until we had time to address it in the next Sprint.
2. Teaspoon, Jasmine, Bundler, Rails Engines
In the project there was the production code in Rails.root along with a dummy test code for starting JavaScript specs in Rails.root/test/dummy/. We are using Teaspoon with Jasmine in a Rails engines. Rails engines tend to create a test/dummy folder in which you could put the code for a test app in which you want your JavaScript specs to be executed. The product code is loaded in the test app and specs are executed.
About 1.5 years ago I did some experiments with this test app and made a few configurations. The goal was to have the production code along with some test app code both loaded in the test environment. In this way Teaspoon and Jasmine would see both the production and the test app code as coming from “production”. I literally remembered what my idea was and what I was trying to achieve. At the end I decided not to use the test app code, but to have only the production code located in Rails.root be loaded in the test environment.
But I forgot to remove the test app code from test/dummy/app/assets/javascripts/
3. Teaspoon Error
The following error occurred. Here “IS” is a JavaScript namespace part of the production code.
jenkins@vpszap6s:~/jobs/is-core Build and Release/workspace/test/dummy$ xvfb-run -a bundle exec rake teaspoon --verbose
Starting the Teaspoon server...
Teaspoon running default suite at http://127.0.0.1:36051/teaspoon/default
ReferenceError: IS is not defined
4. The error and debugging process
Not sure what the error was exactly I tried to identify the difference between my machine and the server machine where the tests was failing. This server machine is called “elvis”.
Bundler version – my version was 2.2.11 and the elvis version was 2.2.14. Tried with upgrade, but it did not resolve the issue.
Chrome driver – since the tests were executed on chrome I saw the chrome driver versions were different. Mine was 86. elvis was with 89. Synced them, the error was still occurring.
Rake version – rake versions were the same
Ruby version – the same
Teaspoon and Jasmine versions – the same
The OS is the same
I could not find any difference between my env and the elvis env.
Turns out the problem was in the loading order. elvis was loading test app code and then production code. My machine was loading production code and then test app code. I could not figure out why. The whole test app code was:
I was curious to continue debugging to see why the two machines were loading the code in a different order, but at the end decided against it. Just removed the dead code in test/dummy/app/assets/javascripts/ext/dummy/dummy.js which I only thought as dead, but it turned out it was affecting our project.
6. Builds passing
Finally the builds were passing.
Conclusion
Dead code my not be that dead after all. It might be loaded even if not used and loading order could differ. Better be safe and remove dead code at all. If we need it that much, we have it in the GIT repo history.
We just migrated Stimulus 1.1.1. to Stimulus 2.0.0 so I decided to share this with our whole team, but I thought this could be useful for the whole community.
The practical cost is that this migration could be done in a few minutes per Stimulus controller. 10-20 controllers – you should be done in less than a day.
Overview of the changes
Here are a few examples of the changes that were committed for a single _form.html.erb
In today’s article I am sharing with our team an example I found while reading parts of the code in one of our platforms where we’ve created a form of ‘cyclic’ references with cancancan merges in Rails Controllers. I wondered for a while how we’ve managed to create it, how to avoid creating them in the future and I share this as an example with our team, but I hope the article could be useful for the whole community.
CourseSectionsAbilities is merged with ContentsAbility which is merged with ContentRefsAbilities which is then merged with CourseSectionAbilities. This means that CourseSectionsAbilities is merged twice and the second merge overrides the first merge.
class CourseSectionsController < CommonController
def current_ability
# This is the first merge
# CoruseSectionsAbility is merged with ContentsAbility.
@current_ability ||= Abilities::CourseSectionsAbility.new(current_user)
.merge(Abilities::ContentsAbility.new(current_user))
end
end
module Abilities
class ContentsAbility
include CanCan::Ability
def initialize user
...
# This is the second merge
# ContentsAbility is merged with ContentRefs ability
merge ContentRefsAbility.new user
end
end
end
module Abilities
class ContentRefsAbility
include CanCan::Ability
def initialize user
# This is the third merge
# ContentRefsAbility is merged with CourseSectionsAbilily.
# This overrides the first merge.
merge CourseSectionsAbility.new(user)
...
end
end
end
How did it happen?
It think it is the classic grow of the code where you solve the problem in the easiest way possible. Initially we had:
class CourseSectionsController < CommonController
def current_ability
# This is the first merge
# CoruseSectionsAbility is merged with ContentsAbility.
@current_ability ||= Abilities::CourseSectionsAbility.new(current_user)
.merge(Abilities::ContentsAbility.new(current_user))
end
end
But then a new requirement about an year after that has come and there is a commit that adds the second merge:
module Abilities
class ContentRefsAbility
include CanCan::Ability
def initialize user
# This is the third merge
# ContentRefsAbility is merged with CourseSectionsAbilily.
# This overrides the first merge.
merge CourseSectionsAbility.new(user)
...
end
end
end
What is the problem?
The problem is that there is a class called ContentRefsAbility that can dependent on everything it wants. It can merge anything inside it with any consideration on what was already merged. It can set it’s own dependencies. This couples the ContentRefsAbility with the places it is used. Because we must take into consideration every place where ContentRefsAbility is used before changing it’s implementation.
How Dependency Injection solves this?
We pass the ability in the constructor
module Abilities
class ContentRefsAbility
include CanCan::Ability
def initialize user, outside_ability
# This is the third merge
# ContentRefsAbility is merged with CourseSectionsAbilily.
# This overrides the first merge.
outside_ability.method1 # we call the method of the outside_ability
...
end
end
end
Instead of creating the ability in the class we pass the dependency from the outside. In this way we can control the dependency and choose different dependencies in different conditions.
The ContentRefsAbility no longer depends on the specific implementation of outside ability, but it depends on the behavior we inject from the outside.
Reply
You must be logged in to post a comment.