Issues/1263 scope assets to project in scratch

app/controllers/api/scratch/assets_controller.rb

@@ -4,28 +4,83 @@ module Api
   module Scratch
     class AssetsController < ScratchController
       include ActiveStorage::SetCurrent
+      include RemixSelection
+      include ScratchRemixCreation
 
       skip_before_action :authorize_user, only: [:show]
-      skip_before_action :check_scratch_feature, only: [:show]
+      prepend_before_action :ensure_project_id_header, only: %i[show create]
+      before_action :load_project_from_header, only: %i[show create]
 
       def show
         filename_with_extension = "#{params[:id]}.#{params[:format]}"
-        scratch_asset = ScratchAsset.find_by!(filename: filename_with_extension)
-        redirect_to scratch_asset.file.url(content_type: scratch_asset.file.content_type), allow_other_host: true
+        authorize! :show, @project_from_header
+
+        scratch_asset = ScratchAsset.find_visible_to_project(
+          project: readable_scratch_asset_project,
+          filename: filename_with_extension
+        )
+        raise ActiveRecord::RecordNotFound, 'Not Found' unless scratch_asset
+
+        redirect_to scratch_asset.file.url(content_type: scratch_asset.response_content_type), allow_other_host: true
       end
 
       def create
-        begin
-          filename_with_extension = "#{params[:id]}.#{params[:format]}"
-          ScratchAsset.find_or_create_by!(filename: filename_with_extension) do |a|
-            a.file.attach(io: request.body, filename: filename_with_extension)
+        project = writable_scratch_asset_project
+        return if performed?
+
+        filename_with_extension = "#{params[:id]}.#{params[:format]}"
+        scratch_asset = ScratchAsset.find_or_initialize_by(
+          project:,
+          filename: filename_with_extension
+        )
+
+        if scratch_asset.new_record?
+          begin
+            scratch_asset.save!
+            scratch_asset.file.attach(io: request.body, filename: filename_with_extension)
+          rescue ActiveRecord::RecordNotUnique
+            logger.info("Scratch asset already created during concurrent upload: #{filename_with_extension}")
+            ScratchAsset.find_by!(project:, filename: filename_with_extension)
           end
-        rescue ActiveRecord::RecordNotUnique => e
-          logger.error(e)
         end
 
         render json: { status: 'ok', 'content-name': params[:id] }, status: :created
       end
+
+      private
+
+      def ensure_project_id_header
+        return if request.headers['X-Project-ID'].present?
+
+        render json: { error: 'X-Project-ID header is required' }, status: :bad_request
+      end
+
+      def load_project_from_header
+        @project_from_header = Project.find_by!(
+          identifier: request.headers['X-Project-ID'],
+          project_type: Project::Types::CODE_EDITOR_SCRATCH
+        )
+      end
+
+      def readable_scratch_asset_project
+        return @project_from_header if can?(:update, @project_from_header)
+
+        remix_for_user(@project_from_header, current_user) || @project_from_header
+      end
+
+      def writable_scratch_asset_project
+        return @project_from_header if can?(:update, @project_from_header)
+
+        authorize! :show, @project_from_header
+
+        remix = remix_for_user(@project_from_header, current_user)
+        if remix
+          authorize! :update, remix
+          return remix
+        end
+
+        create_scratch_remix_from(@project_from_header)
+      end
     end
   end
 end

app/controllers/api/scratch/projects_controller.rb

@@ -3,23 +3,37 @@
 module Api
   module Scratch
     class ProjectsController < ScratchController
+      include RemixSelection
+
       skip_before_action :authorize_user, only: [:show]
-      skip_before_action :check_scratch_feature, only: [:show]
       before_action :load_project, only: %i[show update]
 
       before_action :ensure_create_is_a_remix, only: %i[create]
 
       def show
+        authorize! :show, @project
+
         render json: @project.scratch_component.content
       end
 
       def create
         original_project = load_original_project(source_project_identifier)
-        return render json: { error: I18n.t('errors.admin.unauthorized') }, status: :unauthorized unless current_ability.can?(:show, original_project)
+        authorize! :show, original_project
 
         remix_params = create_params
         return render json: { error: I18n.t('errors.project.remixing.invalid_params') }, status: :bad_request if remix_params.dig(:scratch_component, :content).blank?
 
+        existing_remix = remix_for_user(original_project, current_user)
+        if existing_remix
+          authorize! :update, existing_remix
+
+          scratch_component = existing_remix.scratch_component || existing_remix.build_scratch_component
+          scratch_component.content = scratch_content_params
+          existing_remix.save!
+
+          return render json: { status: 'ok', 'content-name': existing_remix.identifier }, status: :ok
+        end
+
         remix_origin = request.origin || request.referer
 
         result = Project::CreateRemix.call(
@@ -37,6 +51,8 @@ def create
       end
 
       def update
+        authorize! :update, @project
+
         @project.scratch_component&.content = scratch_content_params
         @project.save!
         render json: { status: 'ok' }, status: :ok

app/controllers/concerns/scratch_remix_creation.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module ScratchRemixCreation
+  extend ActiveSupport::Concern
+
+  private
+
+  def create_scratch_remix_from(project)
+    result = Project::CreateRemix.call(
+      params: {
+        identifier: project.identifier,
+        scratch_component: { content: scratch_component_content_for(project) }
+      },
+      user_id: current_user.id,
+      original_project: project,
+      remix_origin: request.origin || request.referer || request.base_url
+    )
+
+    return result[:project] if result.success?
+
+    render json: { error: result[:error] }, status: :bad_request
+    nil
+  end
+
+  def scratch_component_content_for(project)
+    project.scratch_component&.content&.deep_dup || {
+      meta: {},
+      targets: [],
+      monitors: [],
+      extensions: []
+    }
+  end
+end

app/models/project.rb

@@ -86,6 +86,20 @@ def scratch_project?
     project_type == Types::CODE_EDITOR_SCRATCH
   end
 
+  def self_and_ancestor_ids
+    ids = []
+    current_project = self
+    seen_ids = []
+
+    while current_project && seen_ids.exclude?(current_project.id)
+      ids << current_project.id
+      seen_ids << current_project.id
+      current_project = current_project.parent
+    end
+
+    ids
+  end
+
   private
 
   def check_unique_not_null

app/models/scratch_asset.rb

@@ -1,7 +1,50 @@
 # frozen_string_literal: true
 
 class ScratchAsset < ApplicationRecord
-  validates :filename, presence: true, uniqueness: true
-
+  belongs_to :project, optional: true
   has_one_attached :file
+
+  validates :filename, presence: true, uniqueness: { scope: :project_id }
+  validate :belongs_to_scratch_project
+
+  scope :global_assets, -> { where(project_id: nil) }
+  scope :project_assets, -> { where.not(project_id: nil) }
+
+  def self.find_visible_to_project(project:, filename:)
+    lineage_ids = project.self_and_ancestor_ids
+    lineage_rank_by_id = lineage_ids.each_with_index.to_h
+
+    includes(:file_attachment)
+      .where(filename:, project_id: lineage_ids)
+      .to_a
+      .min_by { |asset| lineage_rank_by_id.fetch(asset.project_id) } ||
+      global_assets.includes(:file_attachment).find_by(filename:)
+  end
+
+  def global?
+    project_id.nil?
+  end
+
+  def response_content_type
+    return 'image/svg+xml' if global? && svg?
+    return 'application/octet-stream' if svg?
+
+    file.content_type.presence || 'application/octet-stream'
+  end
+
+  private
+
+  def belongs_to_scratch_project
+    return if project.blank? || belongs_to_scratch_project?
+
+    errors.add(:project, 'must be a Scratch project')
+  end
+
+  def belongs_to_scratch_project?
+    project.scratch_project?
+  end
+
+  def svg?
+    File.extname(filename).casecmp('.svg').zero?
+  end
 end

db/migrate/20260410110000_scope_scratch_assets_to_projects.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+class ScopeScratchAssetsToProjects < ActiveRecord::Migration[7.2]
+  def up
+    add_reference :scratch_assets, :project, null: true, foreign_key: true, type: :uuid
+    remove_index :scratch_assets, :filename
+
+    backfill_legacy_scratch_assets_to_projects
+
+    add_index :scratch_assets,
+              %i[project_id filename],
+              unique: true,
+              where: 'project_id IS NOT NULL',
+              name: 'index_scratch_assets_on_project_id_and_filename'
+
+    add_index :scratch_assets,
+              :filename,
+              unique: true,
+              where: 'project_id IS NULL',
+              name: 'index_scratch_assets_on_global_filename'
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration,
+          'Scratch assets are backfilled onto projects and cannot be safely collapsed back into one global row per filename'
+  end
+
+  private
+
+  def backfill_legacy_scratch_assets_to_projects
+    say_with_time 'Backfilling legacy Scratch assets to projects' do
+      ::ScratchComponent.find_each { |component| backfill_component_assets(component) }
+    end
+  end
+
+  def backfill_component_assets(component)
+    extract_asset_filenames(component.content).uniq.each do |filename|
+      backfill_asset_to_project(filename:, project_id: component.project_id)
+    end
+  end
+
+  def backfill_asset_to_project(filename:, project_id:)
+    return if ::ScratchAsset.exists?(filename:, project_id:)
+
+    legacy_asset = ::ScratchAsset.find_by(filename:, project_id: nil)
+    if legacy_asset
+      legacy_asset.update!(project_id:)
+      return
+    end
+
+    source_asset = ::ScratchAsset.where(filename:).where.not(project_id: nil).first
+    duplicate_asset_for_project(source_asset:, project_id:) if source_asset
+  end
+
+  def extract_asset_filenames(content)
+    case content
+    when Array
+      content.flat_map { |item| extract_asset_filenames(item) }
+    when Hash
+      filenames = []
+      filenames << content['md5ext'] if content['md5ext'].present?
+      content.each_value { |value| filenames.concat(extract_asset_filenames(value)) }
+      filenames
+    else
+      []
+    end
+  end
+
+  def duplicate_asset_for_project(source_asset:, project_id:)
+    attachment = file_attachment_for(source_asset)
+    return unless attachment
+
+    duplicated_asset = ::ScratchAsset.create!(
+      filename: source_asset.filename,
+      project_id:,
+      created_at: source_asset.created_at,
+      updated_at: source_asset.updated_at
+    )
+
+    ::ActiveStorage::Attachment.create!(
+      name: 'file',
+      record_type: 'ScratchAsset',
+      record_id: duplicated_asset.id,
+      blob_id: attachment.blob_id,
+      created_at: attachment.created_at
+    )
+  end
+
+  def file_attachment_for(asset)
+    ::ActiveStorage::Attachment.find_by(record_type: 'ScratchAsset', record_id: asset.id, name: 'file')
+  end
+end

lib/scratch_asset_importer.rb

@@ -29,11 +29,13 @@ def import
   private
 
   def import_asset(asset_name)
-    return if ScratchAsset.exists?(filename: asset_name)
+    return if ScratchAsset.exists?(filename: asset_name, project_id: nil)
 
     sleep(ASSET_FETCHING_DELAY)
     asset = connection.get("#{asset_name}/get/")
-    ScratchAsset.create!(filename: asset_name).file.attach(io: StringIO.new(asset.body), filename: asset_name)
+    ScratchAsset.create!(filename: asset_name, project_id: nil)
+                .file
+                .attach(io: StringIO.new(asset.body), filename: asset_name)
   rescue StandardError => e
     Rails.logger.error("Failed to import asset #{asset_name}: #{e.message}")
   end

spec/factories/scratch_assets.rb

@@ -3,6 +3,7 @@
 FactoryBot.define do
   factory :scratch_asset do
     sequence(:filename) { Random.hex }
+    project { nil }
 
     trait :with_file do
       transient { asset_path { file_fixture('test_image_1.png') } }