From 7c1d2e5368fd0bf7f451f70c1dc2345384ac651e Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Tue, 1 Mar 2022 13:30:00 -0600
Subject: [PATCH] Use optional fields on Synced Folder instead of empty string
 checks

Addresses concerns raised in discussion here
https://github.com/hashicorp/vagrant-ruby/pull/219#discussion_r816966056
and makes it so we don't have to change any plugin code to make things
work.

Depends on https://github.com/hashicorp/vagrant-plugin-sdk/pull/133
---
 internal/core/machine.go                             |  7 ++++---
 .../protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb  |  6 +++---
 plugins/commands/serve/client/target/machine.rb      | 11 ++++++++++-
 plugins/synced_folders/rsync/helper.rb               |  8 ++------
 .../unit/plugins/synced_folders/rsync/helper_test.rb | 12 ------------
 5 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/internal/core/machine.go b/internal/core/machine.go
index 51d88a3dd..674d9336e 100644
--- a/internal/core/machine.go
+++ b/internal/core/machine.go
@@ -231,11 +231,12 @@ func (m *Machine) SyncedFolders() (folders []*core.MachineSyncedFolder, err erro
 
 	folders = []*core.MachineSyncedFolder{}
 	for _, folder := range syncedFolders {
-		if folder.Type == "" {
+		if folder.Type == nil {
 			// TODO: get default synced folder type
-			folder.Type = "virtualbox"
+			defaultType := "virtualbox"
+			folder.Type = &defaultType
 		}
-		plg, err := m.project.basis.component(m.ctx, component.SyncedFolderType, folder.Type)
+		plg, err := m.project.basis.component(m.ctx, component.SyncedFolderType, *folder.Type)
 		if err != nil {
 			return nil, err
 		}
diff --git a/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb b/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb
index 5f7c7aafa..55d5ff082 100644
--- a/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb
+++ b/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb
@@ -776,11 +776,11 @@ Google::Protobuf::DescriptorPool.generated_pool.build do
       optional :config, :message, 3, "google.protobuf.Any"
       optional :create, :bool, 4
       optional :disabled, :bool, 5
-      optional :group, :string, 6
+      proto3_optional :group, :string, 6
       optional :id, :string, 7
       repeated :mount_options, :string, 8
-      optional :owner, :string, 9
-      optional :type, :string, 10
+      proto3_optional :owner, :string, 9
+      proto3_optional :type, :string, 10
     end
     add_message "hashicorp.vagrant.sdk.Vagrantfile.PushConfig" do
       optional :name, :string, 1
diff --git a/plugins/commands/serve/client/target/machine.rb b/plugins/commands/serve/client/target/machine.rb
index 6a0829363..54704cdb7 100644
--- a/plugins/commands/serve/client/target/machine.rb
+++ b/plugins/commands/serve/client/target/machine.rb
@@ -113,7 +113,7 @@ module VagrantPlugins
             folder_protos.map do |fp|
               {
                 plugin: SyncedFolder.load(fp.plugin, broker: broker),
-                folder: fp.folder.to_h,
+                folder: _cleaned_folder_hash(fp.folder),
               }
             end
           end
@@ -122,6 +122,15 @@ module VagrantPlugins
           def uid
             client.uid(Empty.new).uid
           end
+
+          def _cleaned_folder_hash(folder)
+            folder_hash = folder.to_h
+            folder_hash.delete_if do |k, v|
+              hazzer = :"has_#{k}?"
+              folder.respond_to?(hazzer) && !folder.send(hazzer)
+            end
+            folder_hash
+          end
         end
       end
     end
diff --git a/plugins/synced_folders/rsync/helper.rb b/plugins/synced_folders/rsync/helper.rb
index 389ae137d..e3b0ef90b 100644
--- a/plugins/synced_folders/rsync/helper.rb
+++ b/plugins/synced_folders/rsync/helper.rb
@@ -71,12 +71,8 @@ module VagrantPlugins
         end
 
         # Folder options
-        if opts[:owner].to_s == ""
-          opts[:owner] = ssh_info[:username]
-        end
-        if opts[:group].to_s == ""
-          opts[:group] = ssh_info[:username]
-        end
+        opts[:owner] ||= ssh_info[:username]
+        opts[:group] ||= ssh_info[:username]
 
         # set log level
         log_level = ssh_info[:log_level] || "FATAL"
diff --git a/test/unit/plugins/synced_folders/rsync/helper_test.rb b/test/unit/plugins/synced_folders/rsync/helper_test.rb
index 4b65ad7c9..462c44a35 100644
--- a/test/unit/plugins/synced_folders/rsync/helper_test.rb
+++ b/test/unit/plugins/synced_folders/rsync/helper_test.rb
@@ -180,18 +180,6 @@ describe VagrantPlugins::SyncedFolderRSync::RsyncHelper do
 
         subject.rsync_single(machine, ssh_info, opts)
       end
-
-      it "should populate :owner and :group from ssh_info[:username] when values are empty strings" do
-        opts[:owner] = ""
-        opts[:group] = ""
-        ssh_info[:username] = "userfromssh"
-        expect(guest).to receive(:capability).with(:rsync_post, a_hash_including(
-          owner: "userfromssh",
-          group: "userfromssh",
-        ))
-
-        subject.rsync_single(machine, ssh_info, opts)
-      end
     end
 
     context "with rsync_ownership option" do