Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC: Isolate dependency injections by specific pattern #59

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davydovanton
Copy link
Member

@davydovanton davydovanton commented Apr 29, 2019

Hey, I have a idea how to improve injector for scopes in business logic.

I create simple PR for discussing and share this idea

Idea

Imagine a situation when you have an application with 2 different scopes: Accounts and Orders. And in each scope (or domain) you have specific operations and repositories. And in account repo we have a similar method name find:

module Accounts
  # Operations for accounts
  module Operations
    class List
      # ...
    end

    class Show
      # ...
    end
  end

  # Repos for accounts
  module Repositories
    class Accounts
      def find(id)
        # ...
      end
    end
  end
end

module Orders
  # Operation for orders
  module Operations
    class Create
      # ...
    end

    class Checkout
      # ...
    end
  end

  # Repos for orders
  module Repositories
    class Orders
      # ...
    end

    class Accounts
      def find(order_id)
        # ...
      end
    end
  end
end

We put all these classes into a container and now we can fail if someone injects account repo from Accounts to Orders scope:

module Orders
  # Operation for orders
  module Operations
    class Create
      # OK for us
      include Inject['orders.repositories.accounts']

      # BAD but we get a exeption ONLY after first call
      include Inject['accounts.repositories.accounts']

      # ...
    end
  end
end

Important thing

This code (with injection 'accounts.repositories.accounts') raise error only after booting and first real call.


For detecting these situations I introduce allow method with simple regexp pattern:

module Accounts
  Inject = Inject.allow(/\Aaccounts.+/) # allow to inject only "accounts.*" keys

  # ...
end

module Orders
  Inject = Inject.allow(/\Aorders.+/) # allow to inject only "orders.*" keys

  module Operations
    class Create
      include Inject['orders.repositories.accounts'] # OK for us
      include Inject['accounts.repositories.accounts'] # raise error on object initialization step

      # ...
    end
  end

  # ...
end

And now, if we will try to inject 'accounts.repositories.accounts' to Orders::Operations::Create we fail on creating an instance of operation.

Implementation

It's just a proposal of my vision. I'm not sure that allow better name for this. But I'll be happy to discuss it with everyone ❤️

@davydovanton
Copy link
Member Author

@davydovanton davydovanton changed the title Proof of concept idea injection dependencies by specific pattern Proof of concept: Allow to inject dependencies by specific pattern Apr 29, 2019
@davydovanton davydovanton changed the title Proof of concept: Allow to inject dependencies by specific pattern PoC: Isolate dependency injections by specific pattern Apr 29, 2019
end

# @api public
def [](*dependency_names)
raise unless match_by_pattern?(dependency_names)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, it will be better to raise some fixed error, not RuntimeError. I think, it would be great for programmers. WDYT about it?

spec/integration/allow_inject_spec.rb Show resolved Hide resolved
spec/integration/allow_inject_spec.rb Show resolved Hide resolved
@solnic
Copy link
Member

solnic commented Jan 4, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
- Added 4
           

See the complete overview on Codacy

describe "allow" do
before do
module Test
AutoInject = Dry::AutoInject({one: 1, two: 2})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,34 @@
# frozen_string_literal: true

RSpec.describe "allow condition" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# frozen_string_literal: true

RSpec.describe "allow condition" do
describe "allow" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe "allow" do
before do
module Test
AutoInject = Dry::AutoInject({one: 1, two: 2})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants