[39451] remove base64 snapshot data from bcf vp

- https://community.openproject.org/work_packages/39451
- remove base64 data after converted to snapshot attachement and before
storing into database
- added migration to remove snapshot data from current data

db/migrate/20211022143726_remove_snapshot_data.rb

@@ -0,0 +1,12 @@
+class RemoveSnapshotData < ActiveRecord::Migration[6.1]
+  def up
+    execute <<~SQL.squish
+      UPDATE bcf_viewpoints
+      SET json_viewpoint = json_viewpoint #- '{snapshot,snapshot_data}';
+    SQL
+  end
+
+  def down
+    # removed snapshot data cannot be restored
+  end
+end

modules/bim/app/contracts/bim/bcf/viewpoints/create_contract.rb

@@ -1,5 +1,3 @@
-#-- encoding: UTF-8
-
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) 2012-2021 the OpenProject GmbH
@@ -78,7 +76,7 @@ module Bim::Bcf
                                        space_boundaries_visible
                                        openings_visible).freeze
 
-      COLOR_REGEXP = /#([0-9a-f]{2})?[0-9a-f]{6}/
+      COLOR_REGEXP = /#([0-9a-f]{2})?[0-9a-f]{6}/.freeze
 
       WHITELISTED_DIMENSIONS = %w(x y z).freeze
 
@@ -86,7 +84,7 @@ module Bim::Bcf
       attribute :issue
       attribute :snapshot
       attribute :json_viewpoint do
-        validate_json_viewpoint_present
+        validate_json_viewpoint_blank
         validate_json_viewpoint_hash
 
         next if errors.any?
@@ -103,7 +101,7 @@ module Bim::Bcf
         validate_guid
       end
 
-      def validate_json_viewpoint_present
+      def validate_json_viewpoint_blank
         errors.add(:json_viewpoint, :blank) if viewpoint.blank?
       end
 
@@ -119,7 +117,7 @@ module Bim::Bcf
         return unless (sjson = viewpoint['snapshot'])
 
         errors.add(:json_viewpoint, :snapshot_type_unsupported) unless %w(jpg png).include? sjson['snapshot_type']
-        errors.add(:json_viewpoint, :snapshot_data_blank) unless sjson['snapshot_data'].present?
+        errors.add(:json_viewpoint, :snapshot_data_blank) if sjson['snapshot_data'].blank?
       end
 
       def validate_index
@@ -216,13 +214,13 @@ module Bim::Bcf
       end
 
       def invalid_components?(components)
-        return false unless components.present?
+        return false if components.blank?
 
         !components.is_a?(Array) || components.any? { |component| invalid_component?(component) }
       end
 
       def invalid_colorings?(colorings)
-        return false unless colorings.present?
+        return false if colorings.blank?
 
         !colorings.is_a?(Array) || colorings.any? { |coloring| invalid_coloring?(coloring) }
       end

modules/bim/app/models/bim/bcf/viewpoint.rb

@@ -1,3 +1,31 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
 module Bim::Bcf
   class Viewpoint < ActiveRecord::Base
     self.table_name = :bcf_viewpoints

modules/bim/app/services/bim/bcf/viewpoints/create_service.rb

@@ -1,5 +1,3 @@
-#-- encoding: UTF-8
-
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) 2012-2021 the OpenProject GmbH
@@ -31,6 +29,14 @@
 module Bim::Bcf
   module Viewpoints
     class CreateService < ::BaseServices::Create
+      protected
+
+      def persist(service_result)
+        # snapshot base64 data must not get stored
+        service_result.result.json_viewpoint['snapshot']&.delete('snapshot_data')
+
+        super(service_result)
+      end
     end
   end
 end

modules/bim/app/services/bim/bcf/viewpoints/set_attributes_service.rb

@@ -1,5 +1,3 @@
-#-- encoding: UTF-8
-
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) 2012-2021 the OpenProject GmbH
@@ -31,6 +29,8 @@
 module Bim::Bcf
   module Viewpoints
     class SetAttributesService < ::BaseServices::SetAttributes
+      private
+
       def set_attributes(params)
         super
 
@@ -38,7 +38,7 @@ module Bim::Bcf
       end
 
       def set_default_attributes(_params)
-        model.json_viewpoint['guid'] = model.uuid
+        viewpoint['guid'] = model.uuid
       end
 
       def set_snapshot
@@ -46,10 +46,10 @@ module Bim::Bcf
 
         name = "snapshot.#{snapshot_extension}"
         file = OpenProject::Files
-          .create_uploaded_file(name: name,
-                                content_type: snapshot_content_type,
-                                content: snapshot_binary_contents,
-                                binary: true)
+                 .create_uploaded_file(name: name,
+                                       content_type: snapshot_content_type,
+                                       content: snapshot_binary_contents,
+                                       binary: true)
 
         # This might break once the service is also used
         # to update existing viewpoints as the snapshot method will
@@ -60,9 +60,9 @@ module Bim::Bcf
       end
 
       def snapshot_data_complete?
-        model.json_viewpoint['snapshot'] &&
+        viewpoint['snapshot'] &&
           snapshot_extension &&
-          snapshot_base_64 &&
+          snapshot_base64 &&
           snapshot_url_parts.length > 1
       end
 
@@ -70,9 +70,9 @@ module Bim::Bcf
         # Return nil when the extension is not within the specified set
         # which will lead to the snapshot not being created.
         # The contract will catch the error.
-        return unless model.json_viewpoint['snapshot']
+        return unless viewpoint['snapshot']
 
-        case model.json_viewpoint['snapshot']['snapshot_type']
+        case viewpoint['snapshot']['snapshot_type']
         when 'png'
           'image/png'
         when 'jpg'
@@ -81,11 +81,11 @@ module Bim::Bcf
       end
 
       def snapshot_extension
-        model.json_viewpoint['snapshot']['snapshot_type']
+        viewpoint['snapshot']['snapshot_type']
       end
 
-      def snapshot_base_64
-        model.json_viewpoint['snapshot']['snapshot_data']
+      def snapshot_base64
+        viewpoint['snapshot']['snapshot_data']
       end
 
       def snapshot_binary_contents
@@ -93,7 +93,11 @@ module Bim::Bcf
       end
 
       def snapshot_url_parts
-        snapshot_base_64.match(/\Adata:([-\w]+\/[-\w+.]+)?;base64,(.*)/m) || []
+        snapshot_base64.match(/\Adata:([-\w]+\/[-\w+.]+)?;base64,(.*)/m) || []
+      end
+
+      def viewpoint
+        model.json_viewpoint
       end
     end
   end

modules/bim/spec/requests/api/bcf/v2_1/viewpoints_api_spec.rb

@@ -267,6 +267,7 @@ describe 'BCF 2.1 viewpoints resource', type: :request, content_type: :json, wit
 
         params
           .merge(guid: new_viewpoint.uuid)
+          .merge("snapshot" => { "snapshot_type" => "png" })
       end
 
       let(:expected_status) { 201 }
@@ -274,10 +275,10 @@ describe 'BCF 2.1 viewpoints resource', type: :request, content_type: :json, wit
 
     it 'creates the viewpoint with an attachment for the snapshot' do
       expect(Bim::Bcf::Viewpoint.count)
-        .to eql 2
+        .to be 2
 
       expect(Bim::Bcf::Viewpoint.last.attachments.count)
-        .to eql 1
+        .to be 1
     end
 
     context 'lacking permission to see project' do
@@ -331,7 +332,7 @@ describe 'BCF 2.1 viewpoints resource', type: :request, content_type: :json, wit
 
       it 'creates the viewpoint with an attachment for the snapshot' do
         expect(Bim::Bcf::Viewpoint.count)
-          .to eql 2
+          .to be 2
       end
     end