Allow custom course prefix (#800)

angelsl · web-flow · commit b78479e7da78 · 2021-08-05T03:42:31.000+08:00
diff --git a/lib/cadet/assets/assets.ex b/lib/cadet/assets/assets.ex
@@ -9,18 +9,18 @@ defmodule Cadet.Assets.Assets do
                         if(Mix.env() == :test, do: ["testFolder"], else: [])
   @accepted_file_types ~w(.jpg .jpeg .gif .png .wav .mp3 .txt)
 
-  def upload_to_s3(upload_params, course_id, folder_name, file_name) do
+  def upload_to_s3(upload_params, prefix, folder_name, file_name) do
     file_type = Path.extname(file_name)
 
     with :ok <- validate_file_name(file_name),
          :ok <- validate_folder_name(folder_name),
          :ok <- validate_file_type(file_type) do
-      if object_exists?(course_id, folder_name, file_name) do
+      if object_exists?(prefix, folder_name, file_name) do
         {:error, {:bad_request, "File already exists"}}
       else
         file = upload_params.path
 
-        s3_path = "#{prefix()}#{course_id}/#{folder_name}/#{file_name}"
+        s3_path = "#{prefix}#{folder_name}/#{file_name}"
 
         file
         |> Upload.stream_file()
@@ -32,25 +32,29 @@ defmodule Cadet.Assets.Assets do
     end
   end
 
-  def list_assets(course_id, folder_name) do
+  def list_assets(prefix, folder_name) do
+    prefix_len = byte_size(prefix)
+
     case validate_folder_name(folder_name) do
       :ok ->
         bucket()
-        |> S3.list_objects(prefix: "#{prefix()}#{course_id}/" <> folder_name <> "/")
+        |> S3.list_objects(prefix: prefix <> folder_name <> "/")
         |> ExAws.stream!()
-        |> Enum.map(fn file -> file.key end)
+        |> Enum.map(fn file ->
+          binary_part(file.key, prefix_len, byte_size(file.key) - prefix_len)
+        end)
 
       {:error, _} = error ->
         error
     end
   end
 
-  def delete_object(course_id, folder_name, file_name) do
+  def delete_object(prefix, folder_name, file_name) do
     with :ok <- validate_file_name(file_name),
          :ok <- validate_folder_name(folder_name) do
-      if object_exists?(course_id, folder_name, file_name) do
+      if object_exists?(prefix, folder_name, file_name) do
         bucket()
-        |> S3.delete_object("#{prefix()}#{course_id}/#{folder_name}/#{file_name}")
+        |> S3.delete_object("#{prefix}#{folder_name}/#{file_name}")
         |> ExAws.request!()
 
         :ok
@@ -60,11 +64,11 @@ defmodule Cadet.Assets.Assets do
     end
   end
 
-  @spec object_exists?(integer(), binary, binary) :: boolean()
-  def object_exists?(course_id, folder_name, file_name) do
+  @spec object_exists?(integer() | binary(), binary, binary) :: boolean()
+  def object_exists?(prefix, folder_name, file_name) do
     response =
       bucket()
-      |> S3.head_object("#{prefix()}#{course_id}/#{folder_name}/#{file_name}")
+      |> S3.head_object("#{prefix}#{folder_name}/#{file_name}")
       |> ExAws.request()
 
     case response do
@@ -99,5 +103,6 @@ defmodule Cadet.Assets.Assets do
 
   defp bucket, do: :cadet |> Application.fetch_env!(:uploader) |> Keyword.get(:assets_bucket)
 
-  defp prefix, do: :cadet |> Application.fetch_env!(:uploader) |> Keyword.get(:assets_prefix, "")
+  def assets_prefix,
+    do: :cadet |> Application.fetch_env!(:uploader) |> Keyword.get(:assets_prefix, "")
 end
diff --git a/lib/cadet/courses/course.ex b/lib/cadet/courses/course.ex
@@ -17,6 +17,9 @@ defmodule Cadet.Courses.Course do
     field(:source_variant, :string)
     field(:module_help_text, :string)
 
+    # for now, only settable from database
+    field(:assets_prefix, :string, default: nil)
+
     has_many(:assessment_config, AssessmentConfig)
 
     timestamps()
diff --git a/lib/cadet/courses/courses.ex b/lib/cadet/courses/courses.ex
@@ -20,6 +20,7 @@ defmodule Cadet.Courses do
 
   alias Cadet.Assessments
   alias Cadet.Assessments.Assessment
+  alias Cadet.Assets.Assets
 
   @doc """
   Creates a new course configuration, course registration, and sets
@@ -408,4 +409,9 @@ defmodule Cadet.Courses do
     |> Repo.all()
     |> Repo.preload(:uploader)
   end
+
+  @spec assets_prefix(%Course{}) :: binary()
+  def assets_prefix(course) do
+    course.assets_prefix || "#{Assets.assets_prefix()}#{course.id}/"
+  end
 end
diff --git a/lib/cadet_web/admin_controllers/admin_assets_controller.ex b/lib/cadet_web/admin_controllers/admin_assets_controller.ex
@@ -2,12 +2,14 @@ defmodule CadetWeb.AdminAssetsController do
   use CadetWeb, :controller
 
   use PhoenixSwagger
+
   alias Cadet.Assets.Assets
+  alias Cadet.Courses
 
   def index(conn, _params = %{"foldername" => foldername}) do
     course_reg = conn.assigns.course_reg
 
-    case Assets.list_assets(course_reg.course_id, foldername) do
+    case Assets.list_assets(Courses.assets_prefix(course_reg.course), foldername) do
       {:error, {status, message}} -> conn |> put_status(status) |> text(message)
       assets -> render(conn, "index.json", assets: assets)
     end
@@ -18,7 +20,7 @@ defmodule CadetWeb.AdminAssetsController do
     course_reg = conn.assigns.course_reg
     filename = Enum.join(filename, "/")
 
-    case Assets.delete_object(course_reg.course_id, foldername, filename) do
+    case Assets.delete_object(Courses.assets_prefix(course_reg.course), foldername, filename) do
       {:error, {status, message}} -> conn |> put_status(status) |> text(message)
       _ -> conn |> put_status(204) |> text('')
     end
@@ -32,7 +34,12 @@ defmodule CadetWeb.AdminAssetsController do
     course_reg = conn.assigns.course_reg
     filename = Enum.join(filename, "/")
 
-    case Assets.upload_to_s3(upload_params, course_reg.course_id, foldername, filename) do
+    case Assets.upload_to_s3(
+           upload_params,
+           Courses.assets_prefix(course_reg.course),
+           foldername,
+           filename
+         ) do
       {:error, {status, message}} -> conn |> put_status(status) |> text(message)
       resp -> render(conn, "show.json", resp: resp)
     end
diff --git a/lib/cadet_web/admin_controllers/admin_courses_controller.ex b/lib/cadet_web/admin_controllers/admin_courses_controller.ex
@@ -84,7 +84,7 @@ defmodule CadetWeb.AdminCoursesController do
   end
 
   swagger_path :update_course_config do
-    put("/v2/courses/{course_id}/admin/onfig")
+    put("/v2/courses/{course_id}/admin/config")
 
     summary("Updates the course configuration for the specified course")
 
diff --git a/lib/cadet_web/controllers/user_controller.ex b/lib/cadet_web/controllers/user_controller.ex
@@ -291,6 +291,7 @@ defmodule CadetWeb.UserController do
             source_variant(Schema.ref(:SourceVariant), "Source Variant name", required: true)
             module_help_text(:string, "Module help text", required: true)
             assessment_types(:list, "Assessment Types", required: true)
+            assets_prefix(:string, "Assets prefix, used by the game")
           end
 
           example(%{
@@ -303,7 +304,8 @@ defmodule CadetWeb.UserController do
             source_chapter: 1,
             source_variant: "default",
             module_help_text: "Help text",
-            assessment_types: ["Missions", "Quests", "Paths", "Contests", "Others"]
+            assessment_types: ["Missions", "Quests", "Paths", "Contests", "Others"],
+            assets_prefix: "courses-prod/1/"
           })
         end,
       SourceVariant:
diff --git a/lib/cadet_web/views/courses_view.ex b/lib/cadet_web/views/courses_view.ex
@@ -14,7 +14,8 @@ defmodule CadetWeb.CoursesView do
           sourceChapter: :source_chapter,
           sourceVariant: :source_variant,
           moduleHelpText: :module_help_text,
-          assessmentTypes: :assessment_configs
+          assessmentTypes: :assessment_configs,
+          assetsPrefix: :assets_prefix
         })
     }
   end
diff --git a/lib/cadet_web/views/user_view.ex b/lib/cadet_web/views/user_view.ex
@@ -1,6 +1,8 @@
 defmodule CadetWeb.UserView do
   use CadetWeb, :view
 
+  alias Cadet.Courses
+
   def render("index.json", %{
         user: user,
         courses: courses,
@@ -104,7 +106,8 @@ defmodule CadetWeb.UserView do
           enableSourcecast: :enable_sourcecast,
           sourceChapter: :source_chapter,
           sourceVariant: :source_variant,
-          moduleHelpText: :module_help_text
+          moduleHelpText: :module_help_text,
+          assetsPrefix: &Courses.assets_prefix/1
         })
     end
   end
diff --git a/priv/repo/migrations/20210804182725_add_assets_prefix.exs b/priv/repo/migrations/20210804182725_add_assets_prefix.exs
@@ -0,0 +1,9 @@
+defmodule Cadet.Repo.Migrations.AddAssetsPrefix do
+  use Ecto.Migration
+
+  def change do
+    alter table(:courses) do
+      add(:assets_prefix, :string, null: true)
+    end
+  end
+end
diff --git a/test/cadet/assets/assets_test.exs b/test/cadet/assets/assets_test.exs
@@ -7,38 +7,38 @@ defmodule Cadet.Assets.AssetsTest do
   describe "Manage assets" do
     test "accessible folder" do
       use_cassette "aws/model_list_assets#1" do
-        assert Assets.list_assets(1, "testFolder") === [
-                 "1/testFolder/",
-                 "1/testFolder/test.png",
-                 "1/testFolder/test2.png"
+        assert Assets.list_assets(prefix(1), "testFolder") === [
+                 "testFolder/",
+                 "testFolder/test.png",
+                 "testFolder/test2.png"
                ]
       end
     end
 
     test "access another course with 0 folder" do
       use_cassette "aws/model_list_assets#2" do
-        assert Assets.list_assets(2, "testFolder") === []
+        assert Assets.list_assets(prefix(2), "testFolder") === []
       end
     end
 
     test "delete nonexistent file" do
       use_cassette "aws/model_delete_asset#1" do
-        assert Assets.delete_object(1, "testFolder", "test4.png") ===
+        assert Assets.delete_object(prefix(1), "testFolder", "test4.png") ===
                  {:error, {:not_found, "File not found"}}
       end
     end
 
     test "delete ok file" do
       use_cassette "aws/model_delete_asset#2" do
-        assert Assets.delete_object(1, "testFolder", "test.png") === :ok
+        assert Assets.delete_object(prefix(1), "testFolder", "test.png") === :ok
       end
     end
 
     test "upload existing file" do
       use_cassette "aws/model_upload_asset#1" do
         assert Assets.upload_to_s3(
                  build_upload("test/fixtures/upload.png"),
-                 1,
+                 prefix(1),
                  "testFolder",
                  "test2.png"
                ) ===
@@ -50,7 +50,7 @@ defmodule Cadet.Assets.AssetsTest do
       use_cassette "aws/model_upload_asset#2" do
         assert Assets.upload_to_s3(
                  build_upload("test/fixtures/upload.png"),
-                 1,
+                 prefix(1),
                  "testFolder",
                  "test1.png"
                ) ===
@@ -64,4 +64,6 @@ defmodule Cadet.Assets.AssetsTest do
   end
 
   defp bucket, do: :cadet |> Application.fetch_env!(:uploader) |> Keyword.get(:assets_bucket)
+
+  defp prefix(course_id), do: "#{Assets.assets_prefix()}#{course_id}/"
 end
diff --git a/test/cadet_web/admin_controllers/admin_assets_controller_test.exs b/test/cadet_web/admin_controllers/admin_assets_controller_test.exs
@@ -2,8 +2,12 @@ defmodule CadetWeb.AdminAssetsControllerTest do
   use CadetWeb.ConnCase
   use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
 
+  alias Cadet.Courses.Course
+  alias Cadet.Repo
   alias CadetWeb.AdminAssetsController
 
+  import Ecto.Query, only: [where: 2]
+
   setup_all do
     HTTPoison.start()
   end
@@ -196,12 +200,66 @@ defmodule CadetWeb.AdminAssetsControllerTest do
     end
   end
 
+  describe "course with custom assets_prefix" do
+    @tag authenticate: :staff, course_id: 117
+    test "index file", %{conn: conn} do
+      course_id = conn.assigns.course_id
+
+      set_course_prefix(course_id)
+
+      use_cassette "aws/controller_list_assets#2" do
+        conn = get(conn, build_url(course_id, "testFolder"), %{})
+
+        assert json_response(conn, 200) ===
+                 ["testFolder/", "testFolder/test.png", "testFolder/test2.png"]
+      end
+    end
+
+    @tag authenticate: :staff, course_id: 117
+    test "delete file", %{conn: conn} do
+      course_id = conn.assigns.course_id
+
+      set_course_prefix(course_id)
+
+      use_cassette "aws/controller_delete_asset#3" do
+        conn = delete(conn, build_url(course_id, "testFolder/nestedFolder/test2.png"))
+
+        assert response(conn, 204)
+      end
+    end
+
+    @tag authenticate: :staff, course_id: 117
+    test "upload file", %{conn: conn} do
+      course_id = conn.assigns.course_id
+
+      set_course_prefix(course_id)
+
+      use_cassette "aws/controller_upload_asset#3" do
+        conn =
+          post(conn, build_url(course_id, "testFolder/nestedFolder/test.png"), %{
+            "upload" => build_upload("test/fixtures/upload.png")
+          })
+
+        assert json_response(conn, 200) ===
+                 "https://#{bucket()}.s3.amazonaws.com/#{custom_prefix()}testFolder/nestedFolder/test.png"
+      end
+    end
+  end
+
   defp build_url(course_id), do: "/v2/courses/#{course_id}/admin/assets/"
   defp build_url(course_id, url), do: "#{build_url(course_id)}/#{url}"
 
   defp build_upload(path, content_type \\ "image/png") do
     %Plug.Upload{path: path, filename: Path.basename(path), content_type: content_type}
   end
 
+  defp set_course_prefix(course_id) do
+    Course
+    |> where(id: ^course_id)
+    |> Repo.update_all(set: [assets_prefix: custom_prefix()])
+  end
+
   defp bucket, do: :cadet |> Application.fetch_env!(:uploader) |> Keyword.get(:assets_bucket)
+
+  defp custom_prefix, do: "this-is-a-prefix/"
 end
diff --git a/test/cadet_web/controllers/user_controller_test.exs b/test/cadet_web/controllers/user_controller_test.exs
@@ -3,10 +3,10 @@ defmodule CadetWeb.UserControllerTest do
 
   import Cadet.Factory
 
-  alias Cadet.Repo
   alias CadetWeb.UserController
   # alias Cadet.Assessments.{Assessment, Submission}
   alias Cadet.Accounts.{User, CourseRegistration}
+  alias Cadet.{Repo, Courses}
 
   test "swagger" do
     assert is_map(UserController.swagger_definitions())
@@ -100,7 +100,8 @@ defmodule CadetWeb.UserControllerTest do
           "courseName" => "Programming Methodology",
           "sourceChapter" => 1,
           "sourceVariant" => "default",
-          "viewable" => true
+          "viewable" => true,
+          "assetsPrefix" => Courses.assets_prefix(course)
         },
         "assessmentConfigurations" => [
           %{
@@ -226,7 +227,8 @@ defmodule CadetWeb.UserControllerTest do
           "courseName" => "Programming Methodology",
           "sourceChapter" => 1,
           "sourceVariant" => "default",
-          "viewable" => true
+          "viewable" => true,
+          "assetsPrefix" => Courses.assets_prefix(course)
         },
         "assessmentConfigurations" => []
       }
diff --git a/test/fixtures/vcr_cassettes/aws/controller_delete_asset#3.json b/test/fixtures/vcr_cassettes/aws/controller_delete_asset#3.json
diff --git a/test/fixtures/vcr_cassettes/aws/controller_list_assets#1.json b/test/fixtures/vcr_cassettes/aws/controller_list_assets#1.json
diff --git a/test/fixtures/vcr_cassettes/aws/controller_list_assets#2.json b/test/fixtures/vcr_cassettes/aws/controller_list_assets#2.json
diff --git a/test/fixtures/vcr_cassettes/aws/controller_upload_asset#3.json b/test/fixtures/vcr_cassettes/aws/controller_upload_asset#3.json
diff --git a/test/fixtures/vcr_cassettes/aws/model_list_assets#1.json b/test/fixtures/vcr_cassettes/aws/model_list_assets#1.json
