[MNOE-688] Generate relation_id getter and setter for has_one relatio…

…nship

- Introduce HasOneExtension, generating getter and setter for has_one relations, making them saved in relationships, and allowing their retrieving from a relations cache
- Remove useless timestamp method
- Rplace is_integer by is_id that is more semantic
- Try to not rely on parent_organization

api/app/controllers/mno_enterprise/jpi/v1/base_resource_controller.rb

@@ -3,50 +3,61 @@ class Jpi::V1::BaseResourceController < ApplicationController
     before_filter :check_authorization
 
     protected
-
-      def timestamp
-        @timestamp ||= (params[:timestamp] || 0).to_i
+    def is_id?(string)
+      # we consider that it is an id, if it's
+      string.to_i.to_s == string
+    end
+
+    def orga_relation
+      @orga_relation ||= begin
+        id_or_uid = params[:organization_id]
+        organization_field = is_id?(id_or_uid) ? 'id' : 'uid'
+        MnoEnterprise::OrgaRelation.where('user.id' => current_user.id, "organization.#{organization_field}" => id_or_uid).first
       end
-
-      def is_integer?(string)
-        string.to_i.to_s == string
+    end
+
+    def parent_organization_id
+      id_or_uid = params[:organization_id]
+      if is_id?(id_or_uid)
+        id_or_uid
+      else
+        parent_organization.id
       end
+    end
 
-      def parent_organization
-        @parent_organization ||= begin
-          id_or_uid = params[:organization_id]
-          query = is_integer?(id_or_uid) ? id_or_uid : {uid: id_or_uid}
-          o = MnoEnterprise::Organization.includes(:orga_relations, :users).find(query).first
-          ## check that user is in the organization
-          o if o && o.orga_relation(current_user)
-        end
+    def parent_organization
+      @parent_organization ||= begin
+        id_or_uid = params[:organization_id]
+        query = is_id?(id_or_uid) ? id_or_uid : { uid: id_or_uid }
+        MnoEnterprise::Organization.find(query).first
       end
-
-      # Check current user is logged in
-      # Check organization is valid if specified
-      def check_authorization
-        unless current_user
-          render nothing: true, status: :unauthorized
-          return false
-        end
-        if params[:organization_id] && !parent_organization
-          render nothing: true, status: :forbidden
-          return false
-        end
-        true
+    end
+
+    # Check current user is logged in
+    # Check organization is valid if specified
+    def check_authorization
+      unless current_user
+        render nothing: true, status: :unauthorized
+        return false
       end
-
-      def render_not_found(resource, id = params[:id])
-        render json: { errors: {message: "#{resource.titleize} not found (id=#{id})", code: 404, params: params} }, status: :not_found
+      if params[:organization_id] && !orga_relation
+        render nothing: true, status: :forbidden
+        return false
       end
+      true
+    end
 
-      def render_bad_request(attempted_action, issue)
-        issue = issue.full_messages if issue.respond_to?(:full_messages)
-        render json: { errors: {message: "Error while trying to #{attempted_action}: #{issue}", code: 400, params: params} }, status: :bad_request
-      end
+    def render_not_found(resource, id = params[:id])
+      render json: { errors: { message: "#{resource.titleize} not found (id=#{id})", code: 404, params: params } }, status: :not_found
+    end
 
-      def render_forbidden_request(attempted_action)
-        render json: { errors: {message: "Error while trying to #{attempted_action}: you do not have permission", code: 403} }, status: :forbidden
-      end
+    def render_bad_request(attempted_action, issue)
+      issue = issue.full_messages if issue.respond_to?(:full_messages)
+      render json: { errors: { message: "Error while trying to #{attempted_action}: #{issue}", code: 400, params: params } }, status: :bad_request
+    end
+
+    def render_forbidden_request(attempted_action)
+      render json: { errors: { message: "Error while trying to #{attempted_action}: you do not have permission", code: 403 } }, status: :forbidden
+    end
   end
 end

api/lib/mno_enterprise/concerns/controllers/jpi/v1/app_instances_controller.rb

@@ -16,25 +16,25 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::AppInstancesController
   # GET /mnoe/jpi/v1/organization/1/app_instances
   def index
     statuses = MnoEnterprise::AppInstance::ACTIVE_STATUSES.join(',')
-    @app_instances = MnoEnterprise::AppInstance.includes(:app).where(owner_id: parent_organization.id, 'status.in': statuses).to_a.select do |i|
+    @app_instances = MnoEnterprise::AppInstance.includes(:app).where('owner.id': parent_organization_id, 'status.in': statuses).to_a.select do |i|
       can?(:access,i)
     end
   end
 
   # POST /mnoe/jpi/v1/organization/1/app_instances
   def create
-    authorize! :manage_app_instances, parent_organization
-    app_instance = parent_organization.provision_app_instance!(params[:nid])
+    authorize! :manage_app_instances, orga_relation
+    input = { data: { attributes: { app_nid: params[:nid], owner_id: parent_organization_id, owner_type: 'Organization' } } }
+    app_instance = MnoEnterprise::AppInstance.provision!(input)
     MnoEnterprise::EventLogger.info('app_add', current_user.id, 'App added', app_instance)
     head :created
   end
 
   # DELETE /mnoe/jpi/v1/app_instances/1
   def destroy
-    @app_instance = MnoEnterprise::AppInstance.find_one(params[:id])
+    @app_instance = MnoEnterprise::AppInstance.find_one(params[:id], :owner)
     if @app_instance
-      organization = MnoEnterprise::Organization.find_one(@app_instance.owner_id)
-      authorize! :manage_app_instances, organization
+      authorize! :manage_app_instances,  current_user.orga_relation(@app_instance.owner)
       MnoEnterprise::EventLogger.info('app_destroy', current_user.id, 'App destroyed', @app_instance)
       @app_instance = @app_instance.terminate!
     end

api/spec/controllers/mno_enterprise/jpi/v1/app_instances_controller_spec.rb

@@ -14,77 +14,70 @@ module MnoEnterprise
 
     # Stub user and user call
     let(:user) { build(:user) }
+    let(:organization) { build(:organization) }
+    let(:orga_relation) { build(:orga_relation) }
     let!(:current_user_stub) { stub_user(user) }
 
-    # Stub organization and association
-    let!(:organization) {
-      o = build(:organization, orga_relations: [])
-      o.orga_relations << build(:orga_relation, user_id: user.id, organization_id: o.id, role: 'Super Admin')
-      o
-    }
-    before { stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(orga_relations users)) }
+    before { stub_api_v2(:get, '/orga_relations', orga_relation, [], { filter: { 'user.id': user.id, 'organization.id': organization.id }, page: { number: 1, size: 1 } }) }
 
     describe 'GET #index' do
-
-      let(:app_instance) { build(:app_instance, status: 'running' , under_free_trial: false) }
-
-      before { stub_api_v2(:get, '/app_instances', [app_instance], [:app], {filter: {owner_id: organization.id, 'status.in': MnoEnterprise::AppInstance::ACTIVE_STATUSES.join(',')}}) }
-
+      let(:app_instance) { build(:app_instance, status: 'running', under_free_trial: false) }
+      let!(:stub) { stub_api_v2(:get, '/app_instances', [app_instance], [:app], { filter: { 'owner.id': organization.id, 'status.in': MnoEnterprise::AppInstance::ACTIVE_STATUSES.join(',') } }) }
       before { sign_in user }
       subject { get :index, organization_id: organization.id }
 
       it_behaves_like 'jpi v1 protected action'
 
       describe 'all' do
-        it {
+        it do
           subject
-          # TODO: Test that the rendered json is the expected one
-          # expect(assigns(:app_instances)).to eq([app_instance])
-          assert_requested(:get, api_v2_url('/app_instances', [:app], {_locale: I18n.locale, filter: {owner_id: organization.id, 'status.in': MnoEnterprise::AppInstance::ACTIVE_STATUSES.join(',')}}))
-        }
+          expect(subject).to be_successful
+          expect(stub).to have_been_requested
+        end
       end
 
       context 'without access to the app_instance' do
         before { allow(ability).to receive(:can?).with(any_args).and_return(false) }
-        it {
+        it do
           subject
           expect(assigns(:app_instances)).to be_empty
-        }
+        end
       end
     end
 
     describe 'POST #create' do
       before { stub_audit_events }
       let(:app) { build(:app, nid: 'my-app') }
-      let(:app_instance) { build(:app_instance, app: app, owner: organization, owner_id: organization.id) }
+      let(:app_instance) { build(:app_instance, app: app, owner: organization) }
       subject { post :create, organization_id: organization.id, nid: 'my-app' }
       it_behaves_like 'jpi v1 protected action'
+      let!(:stub) { stub_api_v2(:post, '/app_instances/provision', app_instance) }
       before do
-        stub_api_v2(:post, '/app_instances/provision', app_instance)
         sign_in user
       end
 
-      it {
+      it do
         expect(subject).to be_successful
-        assert_requested_api_v2(:post, '/app_instances/provision')
-      }
+        expect(stub).to have_been_requested
+      end
     end
 
     describe 'DELETE #destroy' do
       before { stub_audit_events }
-      let(:app_instance) { build(:app_instance) }
+      let(:app_instance) { build(:app_instance, owner: organization) }
       let(:terminated_app_instance) { build(:app_instance, id: app_instance.id, status: 'terminated') }
-      before { stub_api_v2(:get, "/app_instances/#{app_instance.id}", app_instance)}
-      before { stub_api_v2(:delete, "/app_instances/#{app_instance.id}/terminate", terminated_app_instance)}
-      before { stub_api_v2(:get, "/organizations/#{app_instance.owner_id}")}
+      before { stub_api_v2(:get, "/app_instances/#{app_instance.id}", app_instance, [:owner]) }
+      let!(:stub) { stub_api_v2(:delete, "/app_instances/#{app_instance.id}/terminate", terminated_app_instance) }
+
+
       before { sign_in user }
       subject { delete :destroy, id: app_instance.id }
 
       it_behaves_like 'jpi v1 protected action'
 
       it {
         subject
-        assert_requested_api_v2(:delete, "/app_instances/#{app_instance.id}/terminate")
+        expect(stub).to have_been_requested
         expect(assigns(:app_instance).status).to eq('terminated')
       }
     end

core/app/models/mno_enterprise/base_resource.rb

@@ -3,6 +3,7 @@ module MnoEnterprise
 
   class BaseResource < ::JsonApiClient::Resource
     include ActiveModel::Callbacks
+    include JsonApiClientExtension::HasOneExtension
     self.site = URI.join(MnoEnterprise.api_host, MnoEnterprise.mno_api_v2_root_path).to_s
     self.parser = JsonApiClientExtension::CustomParser
 
@@ -75,6 +76,10 @@ def self.process_custom_result(result)
     end
 
     # == Instance Methods ========================================================
+    # cache of the loaded relations, used in JsonApiClientExtension::HasOneExtension
+    def relations
+      @relations ||= ActiveSupport::HashWithIndifferentAccess.new
+    end
 
     def process_custom_result(result)
       collect_errors(result.errors)

core/app/models/mno_enterprise/orga_relation.rb

@@ -4,8 +4,11 @@ class OrgaRelation < BaseResource
     property :created_at, type: :time
     property :updated_at, type: :time
     # json_api_client map all primary id as string
-    property :organization_id, type: :string
-    property :user_id, type: :string
+
     property :role, type: :string
+
+    has_one :organization
+    has_one :user
+
   end
 end

core/app/models/mno_enterprise/user.rb

@@ -125,7 +125,7 @@ def role_from_id(organization_id)
     end
 
     def orga_relation_from_id(organization_id)
-      self.orga_relations.find { |r| r.organization_id == organization_id }
+      MnoEnterprise::OrgaRelation.where('user.id': id, 'organization.id': organization_id).first
     end
 
     def create_deletion_request!

core/lib/json_api_client_extension/has_one_extension.rb

@@ -0,0 +1,30 @@
+module JsonApiClientExtension::HasOneExtension
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def has_one(attr_name, options = {})
+      class_eval <<-CODE
+        def #{attr_name}_id=(id)
+          ActiveSupport::Deprecation.warn(self.class.name + ".#{attr_name}_id Use relationships instead")
+          association = id ? {data: {type: "#{attr_name.to_s.pluralize}", id: id.to_s}} : nil
+          self.relationships.#{attr_name} = association
+        end
+        def #{attr_name}_id
+          # First we try in the relationship
+          relationship_definitions = self.relationships.try(:#{attr_name})
+          return nil unless relationship_definitions
+          relationship_definitions.dig(:data, :id)
+        end
+        def #{attr_name}=(relation)
+          self.relationships.#{attr_name} = relation
+          relations[:#{attr_name}] = relation
+        end
+        def #{attr_name}
+          relations[:#{attr_name}] ||= super
+        end
+
+      CODE
+      super
+    end
+  end
+end

core/lib/mno_enterprise/concerns/models/ability.rb

@@ -43,20 +43,20 @@ def initialize(user)
          :invite_member,
          :administrate,
          :manage_app_instances,
-         :manage_teams], MnoEnterprise::Organization do |organization|
-      ['Super Admin','Admin'].include? user.role(organization)
+         :manage_teams], MnoEnterprise::OrgaRelation do |orga_relation|
+      ['Super Admin','Admin'].include? orga_relation.role
     end
 
     # To be updated
     # TODO: replace by organization_id, no need to load a full organization
-    can :sync_apps, MnoEnterprise::Organization do |organization|
-      user.role(organization)
+    can :sync_apps, MnoEnterprise::OrgaRelation |orga_relation|
+      !!orga_relation
     end
 
     # To be updated
     # TODO: replace by organization_id, no need to load a full organization
-    can :check_apps_sync, MnoEnterprise::Organization do |organization|
-      user.role(organization)
+    can :check_apps_sync, MnoEnterprise::OrgaRelation do |orga_relation|
+      !!orga_relation
     end
 
     #===================================================
@@ -151,7 +151,7 @@ def impac_abilities(user)
   # Abilities for admin user
   def admin_abilities(user)
     if user.admin_role.to_s.casecmp('admin').zero?
-      can :manage_app_instances, MnoEnterprise::Organization
+      can :manage_app_instances, MnoEnterprise::OrgaRelation
       can :manage_sub_tenant, MnoEnterprise::SubTenant
     end
   end

core/lib/mno_enterprise/concerns/models/app_instance.rb

@@ -10,8 +10,6 @@ module MnoEnterprise::Concerns::Models::AppInstance
     property :created_at, type: :time
     property :updated_at, type: :time
 
-    property :owner_id, type: :string
-
     # delete <api_root>/app_instances/:id/terminate
     custom_endpoint :terminate, on: :member, request_method: :delete
     custom_endpoint :provision, on: :collection, request_method: :post
@@ -21,6 +19,8 @@ module MnoEnterprise::Concerns::Models::AppInstance
     #==============================================================
     ACTIVE_STATUSES = [:running, :stopped, :staged, :provisioning, :starting, :stopping, :updating]
     TERMINATION_STATUSES = [:terminating, :terminated]
+
+    has_one :owner
   end
 
   #==================================================================
@@ -36,12 +36,12 @@ def provision!(input)
   def active?
     status.to_sym.in? ACTIVE_STATUSES
   end
+
   #==================================================================
   # Instance methods
   #==================================================================
 
 
-
   def to_audit_event
     {
       id: id,

core/lib/mno_enterprise/concerns/models/organization.rb

@@ -94,11 +94,6 @@ def add_user!(user, role = 'Member')
     MnoEnterprise::OrgaRelation.create!(organization_id: self.id, user_id: user.id, role: role)
   end
 
-  def provision_app_instance!(app_nid)
-    input = { data: { attributes: { app_nid: app_nid, owner_id: id, owner_type: 'Organization' } } }
-    MnoEnterprise::AppInstance.provision!(input)
-  end
-
   def new_credit_card
     MnoEnterprise::CreditCard.new(owner_id: id, owner_type: 'Organization')
   end

core/lib/mno_enterprise/testing_support/factories/app_instances.rb

@@ -25,7 +25,6 @@
       per_user_licence 1
       app { build(:app, nid: app_nid) }
       sequence(:owner) { |n| build(:organization, id: n.to_s) }
-      sequence(:owner_id, &:to_s)
     end
   end
 end