diff --git a/app/jobs/upload_job.rb b/app/jobs/upload_job.rb index a72de493f..53eb178b8 100644 --- a/app/jobs/upload_job.rb +++ b/app/jobs/upload_job.rb @@ -131,6 +131,11 @@ def categorize_files(files, project_dir, locale, repository, owner) } files.each do |file| + if file.extension == '.sb3' + categories[:components] << scratch_file_component(file, project_dir, locale, repository, owner) + next + end + mime_type = file_mime_type(file) case mime_type @@ -158,11 +163,20 @@ def component(file) { name:, extension:, content:, default: } end + def scratch_file_component(file, project_dir, locale, repository, owner) + name = file.name.chomp(file.extension) + extension = file.extension[1..] + { name:, extension:, io: URI.parse(file_url(file, project_dir, locale, repository, owner)).open } + end + def media(file, project_dir, locale, repository, owner) filename = file.name + { filename:, io: URI.parse(file_url(file, project_dir, locale, repository, owner)).open } + end + + def file_url(file, project_dir, locale, repository, owner) directory = project_dir.name - url = "https://github.com/#{owner}/#{repository}/raw/#{ENV.fetch('GITHUB_WEBHOOK_REF')}/#{locale}/code/#{directory}/#{filename}" - { filename:, io: URI.parse(url).open } + "https://github.com/#{owner}/#{repository}/raw/#{ENV.fetch('GITHUB_WEBHOOK_REF')}/#{locale}/code/#{directory}/#{file.name}" end def repository(payload) diff --git a/app/models/filesystem_project.rb b/app/models/filesystem_project.rb index 3632ba3e2..aa5ce5753 100644 --- a/app/models/filesystem_project.rb +++ b/app/models/filesystem_project.rb @@ -3,7 +3,7 @@ require 'yaml' class FilesystemProject - CODE_FORMATS = ['.py', '.csv', '.txt', '.html', '.css'].freeze + CODE_FORMATS = ['.py', '.csv', '.txt', '.html', '.css', '.sb3'].freeze PROJECTS_ROOT = Rails.root.join('lib/tasks/project_components') PROJECT_CONFIG = 'project_config.yml' @@ -11,7 +11,8 @@ def self.import_all! PROJECTS_ROOT.each_child do |dir| proj_config = YAML.safe_load_file(dir.join(PROJECT_CONFIG).to_s) - files = dir.children.reject { |file| file.basename.to_s == 'project_config.yml' } + files = dir.children.reject { |file| file.basename.to_s == PROJECT_CONFIG } + files = configured_scratch_files(files, proj_config) if proj_config['TYPE'] == Project::Types::CODE_EDITOR_SCRATCH categorized_files = categorize_files(files, dir) project_importer = ProjectImporter.new(name: proj_config['NAME'], identifier: proj_config['IDENTIFIER'], @@ -53,9 +54,18 @@ def self.categorize_files(files, dir) categories end + def self.configured_scratch_files(files, proj_config) + configured_locations = Array(proj_config['COMPONENTS']).pluck('location') + return files if configured_locations.empty? + + files.reject { |file| File.extname(file) == '.sb3' && configured_locations.exclude?(file.basename.to_s) } + end + def self.component(file, dir) name = File.basename(file, '.*') extension = File.extname(file).delete('.') + return { name:, extension:, file_path: dir.join(File.basename(file)).to_s } if extension == 'sb3' + code = File.read(dir.join(File.basename(file)).to_s) default = (File.basename(file) == 'main.py') { name:, extension:, content: code, default: } diff --git a/lib/project_importer.rb b/lib/project_importer.rb index 2669336ba..350583683 100644 --- a/lib/project_importer.rb +++ b/lib/project_importer.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ProjectImporter + class ImportError < StandardError; end + attr_reader :name, :identifier, :images, :videos, :audio, :media, :components, :type, :locale def initialize(**kwargs) @@ -20,6 +22,8 @@ def import! setup_project delete_components create_components + create_scratch_component + create_scratch_assets delete_removed_media attach_media_if_needed @@ -39,16 +43,47 @@ def setup_project end def delete_components + return if project.scratch_project? + project.components.each(&:destroy) end def create_components + return if project.scratch_project? + components.each do |component| + # .sb3 files are only ever imported as a ScratchComponent (see + # create_scratch_component); they carry an :io/:file_path key that is not a + # Component attribute, so skip them here to avoid building invalid rows. + next if component[:extension] == 'sb3' + project_component = Component.new(**component) project.components << project_component end end + def create_scratch_component + return unless project.scratch_project? + + component = components[0] + return unless component&.fetch(:extension, nil) == 'sb3' + + parsed_content = Sb3Parser.new(component: component).parse.fetch(:scratch_component).fetch(:content) + raise ImportError, 'Scratch project content could not be parsed' if parsed_content.blank? + + project.scratch_component = ScratchComponent.new(content: parsed_content) + end + + def create_scratch_assets + return unless project.scratch_project? + + component = components[0] + return unless component&.fetch(:extension, nil) == 'sb3' + + parsed_assets = Sb3Parser.new(component: component).parse.fetch(:assets) + ScratchSb3AssetImporter.import_all(parsed_assets) + end + def delete_removed_media return if removed_media_names.empty? diff --git a/lib/sb3_parser.rb b/lib/sb3_parser.rb new file mode 100644 index 000000000..0dd14b4e9 --- /dev/null +++ b/lib/sb3_parser.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'json' +require 'marcel' +require 'stringio' +require 'zip' + +class Sb3Parser + class MissingProjectJsonError < StandardError; end + class MissingAssetError < StandardError; end + + attr_reader :component, :file_path, :io + + def initialize(component: nil, file_path: nil) + @component = component + @file_path = component&.fetch(:file_path, nil) || file_path + @io = component&.fetch(:io, nil) + end + + def parse + open_zip do |zip_file| + project_json = project_json_entry(zip_file) + content = JSON.parse(project_json.get_input_stream.read) + + output = { + scratch_component: { content: }, + assets: assets(zip_file, extract_asset_names(content)) + } + output + end + end + + private + + def open_zip(&) + return Zip::File.open(file_path, &) if file_path + + io.rewind if io.respond_to?(:rewind) + result = nil + Zip::File.open_buffer(io.read) { |zip_file| result = yield zip_file } + result + end + + def project_json_entry(zip_file) + zip_file.find_entry('project.json') || raise(MissingProjectJsonError, 'project.json not found in SB3 archive') + end + + def extract_asset_names(value) + case value + when Hash + names = [] + names << value['md5ext'] if value['md5ext'].is_a?(String) + value.each_value { |item| names.concat(extract_asset_names(item)) } + names.uniq + when Array + value.flat_map { |item| extract_asset_names(item) }.uniq + else + [] + end + end + + def assets(zip_file, asset_names) + asset_names.map do |asset_name| + entry = zip_file.find_entry(asset_name) || raise(MissingAssetError, "asset #{asset_name} not found in SB3 archive") + asset(entry) + end + end + + def asset(entry) + io = StringIO.new(entry.get_input_stream.read) + content_type = Marcel::MimeType.for(io, name: entry.name) + io.rewind + + { filename: entry.name, io:, content_type: } + end +end diff --git a/lib/scratch_asset_importer.rb b/lib/scratch_asset_importer.rb index 632fba835..8cf5cee21 100644 --- a/lib/scratch_asset_importer.rb +++ b/lib/scratch_asset_importer.rb @@ -45,6 +45,8 @@ def asset end end + private + def create_scratch_asset return if ScratchAsset.global_assets.exists?(filename: asset_name) diff --git a/lib/scratch_sb3_asset_importer.rb b/lib/scratch_sb3_asset_importer.rb new file mode 100644 index 000000000..e18865fe7 --- /dev/null +++ b/lib/scratch_sb3_asset_importer.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'stringio' + +class ScratchSb3AssetImporter + class << self + def import_all(assets) + assets.each do |asset| + new.import(asset) + rescue StandardError + next + end + end + end + + def import(asset) + create_asset(asset.fetch(:filename), asset.fetch(:io).read) + end + + private + + def create_asset(asset_name, content) + return if ScratchAsset.global_assets.exists?(filename: asset_name) + + ScratchAsset.create!(filename: asset_name, project_id: nil, uploaded_user_id: nil) + .file + .attach(io: StringIO.new(content), filename: asset_name) + end +end diff --git a/lib/tasks/project_components/scratch-integration-test-starter/main.sb3 b/lib/tasks/project_components/scratch-integration-test-starter/main.sb3 new file mode 100644 index 000000000..9f2148b52 Binary files /dev/null and b/lib/tasks/project_components/scratch-integration-test-starter/main.sb3 differ diff --git a/lib/tasks/project_components/scratch-integration-test-starter/project_config.yml b/lib/tasks/project_components/scratch-integration-test-starter/project_config.yml new file mode 100644 index 000000000..f6d08cbe2 --- /dev/null +++ b/lib/tasks/project_components/scratch-integration-test-starter/project_config.yml @@ -0,0 +1,3 @@ +NAME: "scratch integration test" +IDENTIFIER: "editor-scratch-testing-starter" +TYPE: "code_editor_scratch" diff --git a/spec/jobs/upload_job_spec.rb b/spec/jobs/upload_job_spec.rb index 61d5bb86a..be69fe875 100644 --- a/spec/jobs/upload_job_spec.rb +++ b/spec/jobs/upload_job_spec.rb @@ -269,6 +269,116 @@ end end + context 'when a scratch project is uploaded' do + let(:scratch_payload) do + { + repository: { name: 'my-amazing-repo', owner: { name: 'me' } }, + commits: [{ added: ['ja-JP/code/scratch-integration-test-starter/main.sb3'], modified: [], removed: [] }] + } + end + let(:scratch_project_json) do + { + targets: [ + { + costumes: [{ md5ext: 'test_image_1.png' }], + sounds: [{ md5ext: 'test_audio_1.mp3' }] + } + ] + } + end + let(:scratch_sb3_body) do + sb3_archive_string( + 'project.json' => scratch_project_json.to_json, + 'test_image_1.png' => sb3_fixture_content('test_image_1.png'), + 'test_audio_1.mp3' => sb3_fixture_content('test_audio_1.mp3') + ) + end + let(:raw_response) do + { + data: { + repository: { + object: { + __typename: 'Tree', + entries: [ + { + name: 'scratch-integration-test-starter', + object: { + __typename: 'Tree', + entries: [ + { + name: 'main.sb3', + extension: '.sb3', + object: { + __typename: 'Blob', + text: nil, + isBinary: true + } + }, + { + name: 'project_config.yml', + extension: '.yml', + object: { + __typename: 'Blob', + text: "name: \"Scratch Integration Test\"\nidentifier: \"scratch-integration-test-starter\"\ntype: \"code_editor_scratch\"\n", + isBinary: false + } + } + ] + } + } + ] + } + } + } + }.deep_stringify_keys + end + + before do + allow(GithubApi::Client).to receive(:query).and_return(graphql_response) + allow(ProjectImporter).to receive(:new).and_call_original + + stub_request(:get, 'https://github.com/me/my-amazing-repo/raw/branches/whatever/ja-JP/code/scratch-integration-test-starter/main.sb3') + .to_return(status: 200, body: scratch_sb3_body, headers: {}) + end + + it 'imports the Scratch project with the sb3 component as io' do + described_class.perform_now(scratch_payload) + + expect(ProjectImporter).to have_received(:new).with( + hash_including( + name: 'Scratch Integration Test', + identifier: 'scratch-integration-test-starter', + type: Project::Types::CODE_EDITOR_SCRATCH, + locale: 'ja-JP', + images: [], + videos: [], + audio: [], + components: [ + hash_including( + name: 'main', + extension: 'sb3', + io: an_object_responding_to(:read) + ) + ] + ) + ) + end + + it 'requests the sb3 file from the correct URL' do + described_class.perform_now(scratch_payload) + + expect(WebMock).to have_requested(:get, 'https://github.com/me/my-amazing-repo/raw/branches/whatever/ja-JP/code/scratch-integration-test-starter/main.sb3').once + end + + it 'saves the Scratch project to the database' do + expect { described_class.perform_now(scratch_payload) }.to change(Project, :count).by(1) + + project = Project.find_by(identifier: 'scratch-integration-test-starter', locale: 'ja-JP') + expect(project.project_type).to eq(Project::Types::CODE_EDITOR_SCRATCH) + expect(project.scratch_component.content).to eq(JSON.parse(scratch_project_json.to_json)) + end + end + context 'when locale is unsupported' do let(:raw_response) { { data: { repository: nil } } } let(:bad_payload) do diff --git a/spec/lib/project_importer_spec.rb b/spec/lib/project_importer_spec.rb index 7d847dc54..a029127a3 100644 --- a/spec/lib/project_importer_spec.rb +++ b/spec/lib/project_importer_spec.rb @@ -114,4 +114,168 @@ expect { importer.import! }.to change { project.reload.audio[0].filename.to_s }.to('my-amazing-audio.mp3') end end + + context 'when a non-scratch project contains an sb3 file' do + let(:project) { Project.find_by(identifier: importer.identifier, user_id: nil, locale: importer.locale) } + let(:importer) do + described_class.new( + name: 'My amazing project', + identifier: 'my-amazing-project', + type: Project::Types::PYTHON, + locale: 'en', + components: [ + { name: 'main', extension: 'py', content: 'print(\'hello\')', default: true }, + { name: 'stray', extension: 'sb3', io: StringIO.new('ignored') } + ] + ) + end + + it 'does not raise when importing' do + expect { importer.import! }.not_to raise_error + end + + it 'skips the sb3 file and only creates the standard components' do + importer.import! + expect(project.components.pluck(:extension)).to eq(['py']) + end + end + + context 'when the project has type code_editor_scratch' do + let(:scratch_project_file) { Tempfile.new(['test_scratch_project', '.sb3']) } + let(:parser) { instance_double(Sb3Parser, parse: parser_result) } + let(:parser_result) do + { + scratch_component: { content: JSON.parse(scratch_project_content.to_json) }, + assets: [ + { filename: 'test_image_1.png', io: StringIO.new(sb3_fixture_content('test_image_1.png')), content_type: 'image/png' }, + { filename: 'test_video_1.mp4', io: StringIO.new(sb3_fixture_content('test_video_1.mp4')), content_type: 'video/mp4' }, + { filename: 'test_audio_1.mp3', io: StringIO.new(sb3_fixture_content('test_audio_1.mp3')), content_type: 'audio/mpeg' } + ] + } + end + let(:importer) do + described_class.new( + name: 'My amazing Scratch project', + identifier: 'my-amazing-scratch-project', + type: Project::Types::CODE_EDITOR_SCRATCH, + locale: 'en', + components: [ + { name: 'main', extension: 'sb3', file_path: scratch_project_file.path } + ] + ) + end + let(:scratch_project_content) do + { + targets: [ + { + costumes: [{ md5ext: 'test_image_1.png' }], + sounds: [{ md5ext: 'test_audio_1.mp3' }], + videos: [{ md5ext: 'test_video_1.mp4' }] + } + ] + } + end + + let(:project) { Project.find_by(identifier: importer.identifier, user_id: nil, locale: importer.locale) } + + before do + allow(Sb3Parser).to receive(:new).and_return(parser) + + scratch_project_file.binmode + scratch_project_file.write( + sb3_archive( + 'project.json' => scratch_project_content.to_json, + 'test_image_1.png' => sb3_fixture_content('test_image_1.png'), + 'test_video_1.mp4' => sb3_fixture_content('test_video_1.mp4'), + 'test_audio_1.mp3' => sb3_fixture_content('test_audio_1.mp3') + ).read + ) + scratch_project_file.flush + end + + after do + scratch_project_file.close + scratch_project_file.unlink + end + + context 'when importing a new scratch project' do + it 'imports the Scratch component content' do + importer.import! + + expect(project.components.count).to eq(0) + expect(project.scratch_component.content).to eq(JSON.parse(scratch_project_content.to_json)) + end + + it 'imports the project assets' do + importer.import! + expect(ScratchAsset.global_assets.where(filename: ['test_image_1.png', 'test_video_1.mp4', 'test_audio_1.mp3']).count).to eq(3) + end + + it 'raises and rolls back the import when the scratch content cannot be parsed' do + allow(parser).to receive(:parse).and_return({ scratch_component: { content: nil }, assets: [] }) + + expect { importer.import! } + .to raise_error(ProjectImporter::ImportError, 'Scratch project content could not be parsed') + + expect(Project.where(identifier: importer.identifier, locale: importer.locale).count).to eq(0) + end + end + + context 'when the scratch project already exists in the database' do + let(:original_scratch_content) do + { targets: ['old target'], monitors: [], extensions: [], meta: {} } + end + let!(:project) do + create( + :project, + identifier: 'my-amazing-scratch-project', + locale: 'en', + project_type: Project::Types::CODE_EDITOR_SCRATCH, + name: 'Old Scratch project name' + ) + end + + before do + create(:scratch_component, project:, content: original_scratch_content) + end + + it 'does not delete existing standard components' do + create(:component, project:, name: 'legacy', extension: 'txt', content: 'keep me') + + expect { importer.import! }.not_to change { project.reload.components.count } + end + + it 'does not create a new project' do + expect { importer.import! }.not_to change(Project, :count) + end + + it 'updates the project name' do + expect { importer.import! }.to change { project.reload.name }.to(importer.name) + end + + it 'updates the scratch component content' do + importer.import! + + expect(project.reload.scratch_component.content).to eq(JSON.parse(scratch_project_content.to_json)) + end + + it 'imports any new assets without duplicating existing ones' do + create(:scratch_asset, :with_file, filename: 'test_image_1.png') + + importer.import! + + expect(ScratchAsset.global_assets.where(filename: ['test_image_1.png', 'test_video_1.mp4', 'test_audio_1.mp3']).count).to eq(3) + end + + it 'rolls back project changes when the scratch content cannot be parsed' do + allow(parser).to receive(:parse).and_return({ scratch_component: { content: nil }, assets: [] }) + + expect { importer.import! } + .to raise_error(ProjectImporter::ImportError, 'Scratch project content could not be parsed') + + expect(project.reload.name).to eq('Old Scratch project name') + expect(project.scratch_component.content).to eq(original_scratch_content.deep_stringify_keys) + end + end + end end diff --git a/spec/lib/sb3_parser_spec.rb b/spec/lib/sb3_parser_spec.rb new file mode 100644 index 000000000..492172d96 --- /dev/null +++ b/spec/lib/sb3_parser_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'sb3_parser' + +RSpec.describe Sb3Parser do + describe '#parse' do + let(:png_content) { sb3_fixture_content('test_image_1.png') } + let(:mp3_content) { sb3_fixture_content('test_audio_1.mp3') } + let(:project_json) do + { + targets: [ + { + costumes: [ + { name: 'cat', md5ext: 'abc123.png' }, + { name: 'duplicate cat', md5ext: 'abc123.png' } + ], + sounds: [ + { name: 'meow', md5ext: 'def456.mp3' } + ] + } + ] + } + end + let(:entries) do + { + 'project.json' => project_json.to_json, + 'abc123.png' => png_content, + 'def456.mp3' => mp3_content + } + end + + it 'parses project.json and referenced assets from component io' do + result = described_class.new(component: { io: sb3_archive(entries) }).parse + + expect(result.fetch(:scratch_component).fetch(:content)).to eq(JSON.parse(project_json.to_json)) + + assets = result.fetch(:assets) + expect(assets.map { |asset| asset.fetch(:filename) }).to contain_exactly('abc123.png', 'def456.mp3') + + png_asset = assets.find { |asset| asset.fetch(:filename) == 'abc123.png' } + expect(png_asset.fetch(:content_type)).to eq('image/png') + expect(png_asset.fetch(:io).read).to eq(png_content) + end + + it 'parses an archive from a file path' do + Tempfile.create(['scratch-project', '.sb3']) do |file| + archive = sb3_archive(entries) + file.binmode + file.write(archive.read) + file.flush + + result = described_class.new(file_path: file.path).parse + + expect(result.fetch(:scratch_component).fetch(:content)).to eq(JSON.parse(project_json.to_json)) + expect(result.fetch(:assets).map { |asset| asset.fetch(:filename) }).to contain_exactly('abc123.png', 'def456.mp3') + end + end + + it 'returns no assets when project.json does not reference any md5ext values' do + archive = sb3_archive('project.json' => { targets: [] }.to_json) + + result = described_class.new(component: { io: archive }).parse + + expect(result.fetch(:assets)).to eq([]) + end + + it 'raises when project.json is missing' do + archive = sb3_archive('abc123.png' => png_content) + + expect do + described_class.new(component: { io: archive }).parse + end.to raise_error(described_class::MissingProjectJsonError, 'project.json not found in SB3 archive') + end + + it 'raises when a referenced asset is missing' do + archive = sb3_archive('project.json' => { targets: [{ costumes: [{ md5ext: 'missing.png' }] }] }.to_json) + + expect do + described_class.new(component: { io: archive }).parse + end.to raise_error(described_class::MissingAssetError, 'asset missing.png not found in SB3 archive') + end + end +end diff --git a/spec/lib/scratch_sb3_asset_importer_spec.rb b/spec/lib/scratch_sb3_asset_importer_spec.rb new file mode 100644 index 000000000..10fdd1c81 --- /dev/null +++ b/spec/lib/scratch_sb3_asset_importer_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'scratch_sb3_asset_importer' + +RSpec.describe ScratchSb3AssetImporter do + describe '.import_all' do + def sb3_asset(filename, content = sb3_fixture_content(filename)) + { filename:, io: StringIO.new(content) } + end + + it 'imports assets from SB3 archive content' do + png_content = sb3_fixture_content('test_image_1.png') + + described_class.import_all([sb3_asset('test_image_1.png', png_content)]) + + scratch_asset = ScratchAsset.find_by(filename: 'test_image_1.png') + expect(scratch_asset).to be_present + expect(scratch_asset).to be_global + expect(scratch_asset.file.download).to eq(png_content) + end + + it 'does nothing if global asset already exists' do + create(:scratch_asset, :with_file, filename: 'test_image_1.png') + + expect do + described_class.import_all([sb3_asset('test_image_1.png')]) + end.not_to change(ScratchAsset, :count) + end + + it 'can import multiple assets' do + described_class.import_all([ + sb3_asset('test_image_1.png'), + sb3_asset('test_audio_1.mp3', sb3_fixture_content('test_audio_1.mp3')) + ]) + + expect(ScratchAsset.find_by(filename: 'test_image_1.png')).to be_present + expect(ScratchAsset.find_by(filename: 'test_audio_1.mp3')).to be_present + end + + it 'still imports a global asset when a project asset already uses the filename' do + project = create(:project, project_type: Project::Types::CODE_EDITOR_SCRATCH, locale: nil, user_id: SecureRandom.uuid) + create(:scratch_component, project:) + create(:scratch_asset, :with_file, filename: 'test_image_1.png', project:) + + expect do + described_class.import_all([sb3_asset('test_image_1.png')]) + end.to change { ScratchAsset.global_assets.where(filename: 'test_image_1.png').count }.by(1) + end + + it 'skips assets that fail to import' do + allow(ScratchAsset).to receive(:create!).and_call_original + allow(ScratchAsset).to receive(:create!).with(filename: 'failing.png', project_id: nil, uploaded_user_id: nil) + + described_class.import_all([ + sb3_asset('failing.png', 'bad'), + sb3_asset('test_image_1.png') + ]) + + expect(ScratchAsset.find_by(filename: 'failing.png')).not_to be_present + expect(ScratchAsset.find_by(filename: 'test_image_1.png')).to be_present + end + end +end diff --git a/spec/support/sb3_archive_helper.rb b/spec/support/sb3_archive_helper.rb new file mode 100644 index 000000000..3250df52d --- /dev/null +++ b/spec/support/sb3_archive_helper.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'zip' +module Sb3ArchiveHelper + def sb3_archive(entries) + Zip::OutputStream.write_buffer do |zip| + entries.each do |name, content| + zip.put_next_entry(name) + zip.write(content) + end + end.tap(&:rewind) + end + + def sb3_archive_string(entries) + sb3_archive(entries).string + end + + def sb3_fixture_content(filename) + Rails.root.join('spec/fixtures/files', filename).binread + end +end + +RSpec.configure do |config| + config.include Sb3ArchiveHelper +end