Resolve "Filesystem restructure after git removal"

.rubocop.yml

-AllCops:
-  TargetRubyVersion: 2.5.1
 GlobalVars:
   AllowedVariables:
+  - $redis
+  - $worker_redis
   - $redis_course_website
 Metrics/LineLength:
-  Max: 120
+  Max: 160
+Metrics/CyclomaticComplexity:
+  Max: 11
+Metrics/PerceivedComplexity:
+  Max: 11
+Metrics/ClassLength:
+  Max: 300
 Metrics/MethodLength:
-  Max: 30
+  Max: 60
\ No newline at end of file
+Metrics/AbcSize:
+  Max: 60
+Metrics/BlockLength:
+  Max: 60
+Style/AsciiComments:
+  Enabled: false
+Metrics/ParameterLists:
+  CountKeywordArgs: false
+AllCops:
+  DisplayCopNames: true
+  Exclude:
+  - '**/*.yml'
+  - 'db/**/*'
+  - 'config/**/*'
+  - 'test/**/*'
+  - 'doc/**/*'
+  - 'bin/**/*'
+  - 'node_modules/**/*'

Gemfile

@@ -31,11 +31,6 @@ gem 'jbuilder', '~> 2.0'
 # bundle exec rake doc:rails generates the API under doc/api.
 gem 'sdoc', '~> 0.4.0', group: :doc
 
-group :development, :test do
-  # Call 'byebug' anywhere in the code to stop execution and get a debugger console
-  gem 'byebug'
-end
-
 group :development do
   # Access an IRB console on exception pages or by using <%= console %> in views
   gem 'web-console', '~> 3.5.1'

app/controllers/enotes_controller.rb

@@ -12,12 +12,13 @@ class EnotesController < ApplicationController
   attr_accessor(:enotes,
                 :pdf_url,
                 :kind_query)
+
   def index
     kind = params[:kind]
 
     get_enotes(kind)
 
-    ap @enotes
+    Rails.logger.debug @enotes.ai
 
     respond_to do |format|
       format.html { render layout: false }
@@ -28,10 +29,10 @@ class EnotesController < ApplicationController
   def show
     kind = params[:kind]
 
-    enote_path = 'enotes'
-    enote_path = 'enotes-staging' if kind.eql? 'staging'
+    enote_path = 'sharelatex-public'
+    enote_path = 'sharelatex-private' if kind.eql? 'staging'
 
-    @pdf_url = File.join('/filemanager', enote_path, my_course_id, params[:enote] + '.pdf')
+    @pdf_url = File.join('/filemanager', my_course_id, enote_path, params[:enote] + '.pdf')
 
     render :show, layout: 'enote'
   end

app/models/enotes.rb

@@ -2,23 +2,18 @@
 
 require 'enote_titleize'
 
+##
+# Cacher for eNotes
 class Enotes < EnotesBase
   include Singleton
 
-  def self.menuize(enote)
-    enote.enote_titleize
-  end
-
-  def redis_key(course)
-    "enotes_#{course}"
+  def initialize
+    super
+    @filemanager_kind = :enotes
   end
 
-  def get_file_list(course)
-    DTUFileManagement::FileManager.instance.enotes_files[course] || []
-  end
-
-  def get_root_path(course)
-    Pathname.new(DTUFileManagement::FileManager.instance.enotes_folder(course))
+  def self.menuize(enote)
+    enote.enote_titleize
   end
 
   def suffix

app/models/enotes_base.rb

+# frozen_string_literal: true
+
 require 'enote_titleize'
 
+##
+# Base class for all enotes cachers
+# TODO: this and DTU Filemanager should be merged in some way.
 class EnotesBase
   def initialize
-    refresh_all
+    @file_manager = DTUFileManagement::FileManager.instance
   end
 
   def get(course)
-    return empty_enotes unless $redis_course_website
+    unless $redis_course_website
+      Rails.logger.warn "No course redis! Returning empty enotes."
+      return empty_enotes
+    end
 
-    enotes_json = $redis_course_website.get(redis_key(course))
+    key = redis_key(course)
+    enotes_json = $redis_course_website.get(key)
 
     if enotes_json.nil?
+      Rails.logger.debug "Refreshing enotes from disc"
       enotes = refresh(course)
     else
-      enotes = (JSON.load enotes_json).symbolize_keys!
+      Rails.logger.debug "Using redis enotes from #{key}"
+      enotes = (JSON.parse enotes_json).symbolize_keys!
       enotes[:enote_stats].map(&:symbolize_keys!)
     end
     enotes
@@ -42,30 +53,42 @@ class EnotesBase
     enote_root = get_root_path course
 
     rel_paths = course_enote_info[:enote_files].map { |file| Pathname.new(file).relative_path_from(enote_root); }
-    course_enote_info[:enote_files] = rel_paths.select {|file| (File.dirname(file).eql? ".") && (File.extname(file).eql? ".pdf")}
+    course_enote_info[:enote_files] = rel_paths.select { |file| (File.dirname(file).eql? '.') && (File.extname(file).eql? '.pdf') }
 
-    course_enote_info[:enotes] = course_enote_info[:enote_files].map {|m| File.basename(m, ".pdf")} if course_enote_info[:enote_files]
+    course_enote_info[:enotes] = course_enote_info[:enote_files].map { |m| File.basename(m, '.pdf') } if course_enote_info[:enote_files]
     course_enote_info[:enotes].each { |enote| course_enote_info[:enote_titles][Enotes.menuize enote] = enote }
 
     course_enote_info[:enote_dirs] = rel_paths.map { |file| File.dirname(file) }
 
-    Rails.logger.debug "Enote info for #{course}: #{course_enote_info.ai}"
-    Rails.logger.info "Enote titles for #{course}: #{course_enote_info[:enote_titles].ai}"
+    Rails.logger.debug "Enote info for #{course}/#{@filemanager_kind}: #{course_enote_info.ai}"
+    Rails.logger.info "Enote titles for #{course}/#{@filemanager_kind}: #{course_enote_info[:enote_titles].ai}"
 
-    $redis_course_website.set(redis_key(course), course_enote_info.to_json) if $redis_course_website
+    $redis_course_website&.set(redis_key(course), course_enote_info.to_json)
 
     course_enote_info
   end
 
+  def get_file_list(course)
+    @file_manager.update
+    @file_manager.files_for(@filemanager_kind, course) || []
+  end
+
+  def get_root_path(course)
+    Pathname.new(@file_manager.folder_for(@filemanager_kind, course))
+  end
+
+  def redis_key(course)
+    "#{@filemanager_kind}_#{course}"
+  end
+
   private
 
   def empty_enotes
     {
-        :enotes => [],
-        :enote_titles => {},
-        :enote_files => [],
-        :enote_dirs => []
+      enotes: [],
+      enote_titles: {},
+      enote_files: [],
+      enote_dirs: []
     }
   end
 end
-

app/models/enotes_staging.rb

+# frozen_string_literal: true
+
 require 'enote_titleize'
 
+##
+# Enotes cacher for stating
 class EnotesStaging < EnotesBase
-
   include Singleton
 
-  def self.menuize(enote)
-    enote.enote_titleize
-  end
-
-  def redis_key(course)
-    "enotes_staging_#{course}"
+  def initialize
+    super
+    @filemanager_kind = :enotes_staging
   end
 
-  def get_file_list(course)
-    # TODO: Really add this to filemanager.  Or alternatively refactor FileManager back into the course website
-
-    root = get_root_path course
-    files = Dir.glob(File.join(root, "**/*"))
-
-    enote_files = []
-    files.each do |file|
-      next unless File.file? file
-      enote_files.push file
-    end
-
-    enote_files
-  end
-
-  def get_root_path(course)
-    Pathname.new(DTUFileManagement::FileManager.instance.enotes_folder(course).gsub('enotes', 'enotes-staging'))
+  def self.menuize(enote)
+    enote.enote_titleize
   end
 
   def suffix
-    "EnotesStaging"
+    'EnotesStaging'
   end
 end
-

app/models/website_course_config.rb

@@ -19,11 +19,11 @@ class WebsiteCourseConfig
   end
 
   def pages_source_path
-    File.join (ENV['DTU_DATA_ROOT'] || '/data'), 'website-raw', @course
+    File.join Rails.configuration.dtu_data[:root_path], 'content', @course, 'pages'
   end
 
   def pages_compiled_path
-    File.join (ENV['DTU_DATA_ROOT'] || '/data'), 'website-public', "assets/#{@course}"
+    File.join Rails.configuration.dtu_data[:root_path], 'website-public', "assets/#{@course}"
   end
 
   def page_external_url(page)

app/services/pages_service.rb

@@ -121,7 +121,9 @@ class PagesService
     expected_pages = Menu.get(@course).expected_menu_pages
     expected_pages.each do |page|
       expected_file = Pages.get_file_name(compiled_folder, page)
+
       next if File.exist? expected_file
+
       Errors.set_page(@course, expected_file, [
                         { file: expected_file, refer_page: page, line: 0, text: 'Missing file referenced in Menu' }
                       ])
@@ -260,12 +262,13 @@ class PagesService
 
     begin
       if page.eql? Menu.PageName
-        menu, errors = Menu.refresh @course
+        _menu, errors = Menu.refresh @course
         return errors
       end
 
       uploads = (DTUFileManagement::FileManager.instance.uploads_files[@course] || []).map do |u|
-        u.gsub("/filemanager/uploads/#{@course}/", '')
+        u.gsub(DTUFileManagement::FileManager.instance.get_local_folder_path(:uploads, @course), '')
+         .delete_prefix('/')
       end
       valid_links = {
         '' => Pages.get(@course).valid_page_names,
@@ -297,10 +300,8 @@ class PagesService
   end
 
   def read_lockfile(lock_path)
-    if File.exist? lock_path
-      lock_user = File.read lock_path
-      lock_user
-    end
+    return nil unless File.exist? lock_path
+    File.read lock_path
   end
 
   def all_current_edits

app/services/update_service.rb

@@ -3,6 +3,8 @@
 require 'open3'
 require 'fileutils'
 
+##
+# Updates
 class UpdateService
   attr_accessor(
     :course
@@ -80,13 +82,14 @@ class UpdateService
     pub_path = Enotes.instance.get_root_path(course)
     staging_path = EnotesStaging.instance.get_root_path(course)
 
+    Rails.logger.info "Publishing #{staging_path} to #{pub_path}"
     FileUtils.rm_rf(Dir.glob(File.join(pub_path, '**/*')))
 
     Dir.glob(File.join(staging_path, '**/*')).each do |file|
       dest = File.join(pub_path, file.gsub(staging_path.to_s, ''))
       # will raise if this fails
       # FileUtils.move file, dest
-      FileUtils.mv(file, dest, :verbose => true, :force => true)
+      FileUtils.mv(file, dest, verbose: true, force: true)
     end
 
     update_enotes
@@ -96,29 +99,27 @@ class UpdateService
     DTUFileManagement::FileManager.instance.update
     Enotes.instance.refresh course
     EnotesStaging.instance.refresh course
-    Rails.logger.info(DTUFileManagement::FileManager.instance.enotes_files)
+    Rails.logger.info(DTUFileManagement::FileManager.instance.enotes_files.ai)
   end
 
-  def update_filemanager(user = DTUAuth2::CachedAuthorizationManager.system_user)
+  def update_filemanager(_user = DTUAuth2::CachedAuthorizationManager.system_user)
     Rails.logger.info('Updating FileManager')
 
     # TODO: in the new world order of the filemanager, does anything happen here?
-=begin
-    sync_website_repo(:files, user)
-
-    fm_folder = DTUFileManagement::FileManager.instance.uploads_folder(@course)
-    gitinfo = WebsiteConfig.get.courses[@course].files_git_repo_info
-
-    Rails.logger.info("emptying #{fm_folder}")
-
-    FileUtils.rm_rf(Dir.glob(fm_folder + '/*'))
-
-    Rails.logger.info("copying #{gitinfo.local_repo_folder} to #{fm_folder}")
-
-    FileUtils.copy_entry gitinfo.local_repo_folder, fm_folder # , :remove_destination => true
-
-    FileUtils.rm_rf(Dir.glob(fm_folder + '/.git'))
-=end
+    #     sync_website_repo(:files, user)
+    #
+    #     fm_folder = DTUFileManagement::FileManager.instance.uploads_folder(@course)
+    #     gitinfo = WebsiteConfig.get.courses[@course].files_git_repo_info
+    #
+    #     Rails.logger.info("emptying #{fm_folder}")
+    #
+    #     FileUtils.rm_rf(Dir.glob(fm_folder + '/*'))
+    #
+    #     Rails.logger.info("copying #{gitinfo.local_repo_folder} to #{fm_folder}")
+    #
+    #     FileUtils.copy_entry gitinfo.local_repo_folder, fm_folder # , :remove_destination => true
+    #
+    #     FileUtils.rm_rf(Dir.glob(fm_folder + '/.git'))
     Rails.logger.info('Updating file manager')
 
     DTUFileManagement::FileManager.instance.update
@@ -131,12 +132,12 @@ class UpdateService
       podcast_record_path if PodcastService.new_podcasts? podcast_record_path
     end.reject(&:nil?)
 
-    new_podcasts_href = new_podcasts.map { |podcast_json_path|
+    new_podcasts_href = new_podcasts.map do |podcast_json_path|
       (PodcastService.load_file podcast_json_path)&.[]('href')
-    }
+    end
 
     abort("new podcasts found #{new_podcasts.ai}, #{new_podcasts_href.ai}") unless new_podcasts.empty?
 
-    return false
+    false
   end
 end

config/initializers/filemanager.rb

-unless ENV.has_key? "PRECOMPILE"
+# frozen_string_literal: true
+
+unless ENV.key? 'PRECOMPILE'
   courses = WebsiteConfig.get.courses.keys
-  Enotes.instance.refresh_all
   file_manager = DTUFileManagement::FileManager.instance
+  # file_manager.dtu_file_manager_root = WebsiteConfig.config_root
   file_manager.update
-  for course in courses do
+  Enotes.instance.refresh_all
+
+  courses.each do |course|
     Rails.logger.info "Enotes for #{course} from #{file_manager.enotes_folder(course)}:\n#{file_manager.enotes_files.ai}"
     Rails.logger.info "Uploads for #{course} from #{file_manager.uploads_folder(course)}:\n#{file_manager.uploads_files.ai}"
   end

dtu-core @ 601f75c4

-Subproject commit 7c287b95548d0a3fc38aad0e7da26807f3d90643
+Subproject commit 601f75c4374ceceae3ba57e2f8bb5a9d81e658d2

spec/controllers/admin/admin_enotes_controller_spec.rb

@@ -5,15 +5,18 @@ require 'fakefs/safe'
 require 'fakefs/spec_helpers'
 
 RSpec.describe Admin::AdminEnotesController, type: :controller do
-
   describe 'GET #publish_enotes' do
-    DTUAuth2.populate Rails.root.join('spec', 'files', 'config').to_s, Rails.root.join('spec', 'files', 'user_config').to_s
+    config_path = Rails.root.join('spec', 'files', 'config').to_s
+    user_config_path = Rails.root.join('spec', 'files', 'user_config').to_s
+    DTUAuth2.populate config_path, user_config_path
     admin_user = DTUAuth2::CachedAuthorizationManager.user_by_id('testadmin')
-    author_user = DTUAuth2::CachedAuthorizationManager.user_by_id('testauthor')
-    student_user = DTUAuth2::CachedAuthorizationManager.user_by_id('teststudent')
+    # author_user = DTUAuth2::CachedAuthorizationManager.user_by_id('testauthor')
+    # student_user = DTUAuth2::CachedAuthorizationManager.user_by_id('teststudent')
 
     before(:each) do
-      DTUAuth2.populate Rails.root.join('spec', 'files', 'config').to_s, Rails.root.join('spec', 'files', 'user_config').to_s
+      config_path = Rails.root.join('spec', 'files', 'config').to_s
+      user_config_path = Rails.root.join('spec', 'files', 'user_config').to_s
+      DTUAuth2.populate config_path, user_config_path
     end
 
     it 'returns redirect to home for student' do
@@ -24,19 +27,23 @@ RSpec.describe Admin::AdminEnotesController, type: :controller do
       #      Enotes.instance.get(@test_course_id)
       #      EnotesStaging.instance.get(@test_course_id)
       FakeFS do
-        FakeFS::FileSystem.clone(File.join(Rails.root, 'app/models')) # https://github.com/fakefs/fakefs/issues/136
-        FakeFS::FileSystem.clone(File.join(Rails.root, 'app/views')) # https://github.com/fakefs/fakefs/issues/136
-        FakeFS::FileSystem.clone(File.join(Rails.root, 'app/services')) # https://github.com/fakefs/fakefs/issues/136
-        FakeFS::FileSystem.clone(File.join(Rails.root, 'spec/files'))
-        staging_files_before = Dir.glob(File.join(Rails.root, 'spec', 'files', 'enotes-staging', @test_course_id, '**/*'))
+        # these 3:
+        # https://github.com/fakefs/fakefs/issues/136
+        FakeFS::FileSystem.clone(File.join(Rails.root, 'app', 'models'))
+        FakeFS::FileSystem.clone(File.join(Rails.root, 'app', 'views'))
+        FakeFS::FileSystem.clone(File.join(Rails.root, 'app', 'services'))
+        content_path = File.join(Rails.root, 'spec', 'files', 'content', @test_course_id)
+        FakeFS::FileSystem.clone(File.join(Rails.root, 'spec', 'files'))
+        staging_files_before = Dir.glob(File.join(content_path, 'sharelatex-private', '**/*'))
 
         get :publish_enotes, params: { course_id: @test_course_id, format: :json }
 
         expect(response).to have_http_status(:ok)
-        staging_files_after = Dir.glob(File.join(Rails.root, 'spec', 'files', 'enotes-staging', @test_course_id, '**/*'))
-        published_files_after = Dir.glob(File.join(Rails.root, 'spec', 'files', 'enotes', @test_course_id, '**/*'))
+        staging_files_after = Dir.glob(File.join(content_path, 'sharelatex-private', '**/*'))
+        published_files_after = Dir.glob(File.join(content_path, 'sharelatex-public', '**/*'))
         expect(staging_files_after).to eq([])
-        expect(published_files_after.map { |file| File.basename(file) }).to eq(staging_files_before.map { |file| File.basename(file) })
+        basename_compare = ->(file) { File.basename(file) }
+        expect(published_files_after.map(&basename_compare)).to eq(staging_files_before.map(&basename_compare))
       end
     end
   end

spec/spec_helper.rb

@@ -105,7 +105,7 @@ RSpec.configure do |config|
     courses = WebsiteConfig.get.courses.keys
 
     file_manager = DTUFileManagement::FileManager.instance
-    file_manager.dtu_file_manager_root = File.join Rails.root, 'spec/files'
+    file_manager.dtu_file_manager_root = File.join Rails.root, 'spec/files/content'
     file_manager.update
 
     courses.each do |course|