Replacing Newtonsoft.Json dependency with System.Text.Json + JsonPath…

….Net in GarnetJSON (#815)

* wip

* wip

* wip

* wip

* fix

* wip

* wip

* Added synchronization

* test

* fix

* ngtttttt "fix"

This reverts commit 4acd2c9.

* fix

* Added JSON path parse verification

Directory.Packages.props

@@ -7,7 +7,7 @@
     <PackageVersion Include="BenchmarkDotNet" Version="0.14.0" />
     <PackageVersion Include="BenchmarkDotNet.Diagnostics.Windows" Version="0.13.12" />
     <PackageVersion Include="CommandLineParser" Version="2.9.1" />
-    <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
+    <PackageVersion Include="JsonPath.Net" Version="1.1.6" />
     <PackageVersion Include="NLua" Version="1.7.3" />
     <PackageVersion Include="NUnit" Version="4.1.0" />
     <PackageVersion Include="NUnit3TestAdapter" Version="4.6.0" />

libs/common/FileUtils.cs

@@ -6,6 +6,7 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Runtime.Loader;
 using System.Security;
 using System.Text;
 using System.Text.RegularExpressions;
@@ -17,23 +18,27 @@ namespace Garnet.common
     /// </summary>
     public static class FileUtils
     {
+        static object AssemblyLoadLock = new();
+
         /// <summary>
         /// Receives a list of paths and searches for files matching the extensions given in the input
         /// </summary>
         /// <param name="paths">Paths to files or directories</param>
         /// <param name="extensions">Extensions to match files (defaults to any)</param>
+        /// <param name="ignoreFileNames">File names to ignore</param>
         /// <param name="searchOption">In case path is a directory, determines whether to search only top directory or all subdirectories</param>
         /// <param name="files">Files that match the extensions</param>
         /// <param name="errorMessage">Error message, if applicable</param>
         /// <returns>True if successfully enumerated all directories</returns>
-        public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string> files, out string errorMessage, string[] extensions = null, SearchOption searchOption = SearchOption.TopDirectoryOnly)
+        public static bool TryGetFiles(IEnumerable<string> paths, out string[] files, out string errorMessage, string[] extensions = null, IEnumerable<string> ignoreFileNames = null, SearchOption searchOption = SearchOption.TopDirectoryOnly)
         {
             var validExtensionPattern = "^\\.[A-Za-z0-9]+$";
             var anyExtension = false;
 
             errorMessage = string.Empty;
             var sbErrorMessage = new StringBuilder();
             files = null;
+            var ignoreFiles = ignoreFileNames == null ? new HashSet<string>() : [.. ignoreFileNames];
 
             string extensionPattern = null;
             if (extensions == null || extensions.Length == 0)
@@ -63,7 +68,8 @@ public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string
             {
                 if (File.Exists(path))
                 {
-                    if (anyExtension || Regex.IsMatch(path, extensionPattern))
+                    if ((anyExtension || Regex.IsMatch(path, extensionPattern)) &&
+                        !ignoreFiles.Contains(Path.GetFileName(path)))
                     {
                         tmpFiles.Add(path);
                     }
@@ -92,7 +98,8 @@ public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string
 
                 foreach (var filePath in filePaths)
                 {
-                    if (anyExtension || Regex.IsMatch(filePath, extensionPattern))
+                    if ((anyExtension || Regex.IsMatch(filePath, extensionPattern)) &&
+                        !ignoreFiles.Contains(Path.GetFileName(path)))
                     {
                         tmpFiles.Add(filePath);
                     }
@@ -103,7 +110,7 @@ public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string
             if (!errorMessage.IsNullOrEmpty())
                 return false;
 
-            files = tmpFiles;
+            files = [.. tmpFiles];
             return true;
         }
 
@@ -144,23 +151,33 @@ public static bool TryLoadAssemblies(IEnumerable<string> assemblyPaths, out IEnu
             loadedAssemblies = null;
 
             var tmpAssemblies = new List<Assembly>();
-            foreach (var path in assemblyPaths)
+
+            lock (AssemblyLoadLock)
             {
-                Assembly assembly;
-                try
+                foreach (var path in assemblyPaths)
                 {
-                    var data = File.ReadAllBytes(path);
-                    assembly = Assembly.Load(data);
-                }
-                catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException ||
-                                           ex is NotSupportedException || ex is BadImageFormatException ||
-                                           ex is SecurityException)
-                {
-                    sbErrorMessage.AppendLine($"Unable to load assembly from path: {path}. Error: {ex.Message}");
-                    continue;
-                }
+                    Assembly assembly;
+                    try
+                    {
+                        assembly = AssemblyLoadContext.Default.LoadFromAssemblyPath(path);
+                    }
+                    catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException ||
+                                               ex is NotSupportedException || ex is BadImageFormatException ||
+                                               ex is SecurityException)
+                    {
+                        if (ex is FileLoadException && ex.Message == "Assembly with same name is already loaded")
+                        {
+                            var assemblyName = AssemblyName.GetAssemblyName(path).Name;
+                            tmpAssemblies.Add(AssemblyLoadContext.Default.Assemblies.First(a => a.GetName().Name == assemblyName));
+                            continue;
+                        }
+
+                        sbErrorMessage.AppendLine($"Unable to load assembly from path: {path}. Error: {ex.Message}");
+                        continue;
+                    }
 
-                tmpAssemblies.Add(assembly);
+                    tmpAssemblies.Add(assembly);
+                }
             }
 
             errorMessage = sbErrorMessage.ToString();

libs/server/Module/ModuleUtils.cs

@@ -14,25 +14,39 @@ namespace Garnet.server
 {
     public class ModuleUtils
     {
+        /// <summary>
+        /// Load assemblies from specified binary paths
+        /// </summary>
+        /// <param name="binaryPaths">Source paths for assemblies (can be either files or directories)</param>
+        /// <param name="allowedExtensionPaths">List of allowed paths for loading assemblies from</param>
+        /// <param name="allowUnsignedAssemblies">True if loading unsigned assemblies is allowed</param>
+        /// <param name="loadedAssemblies">Loaded assemblies</param>
+        /// <param name="errorMessage">Error message</param>
+        /// <param name="ignoreFileNames">File names to ignore (optional)</param>
+        /// <param name="searchOption">In case path is a directory, determines whether to search only top directory or all subdirectories</param>
+        /// <param name="ignoreAssemblyLoadErrors">False if method should return an error when at least one assembly was not loaded correctly (false by default)</param>
+        /// <returns></returns>
         public static bool LoadAssemblies(
             IEnumerable<string> binaryPaths,
             string[] allowedExtensionPaths,
             bool allowUnsignedAssemblies,
             out IEnumerable<Assembly> loadedAssemblies,
-            out ReadOnlySpan<byte> errorMessage)
+            out ReadOnlySpan<byte> errorMessage,
+            string[] ignoreFileNames = null,
+            SearchOption searchOption = SearchOption.AllDirectories,
+            bool ignoreAssemblyLoadErrors = false)
         {
             loadedAssemblies = null;
             errorMessage = default;
 
             // Get all binary file paths from inputs binary paths
-            if (!FileUtils.TryGetFiles(binaryPaths, out var files, out _, [".dll", ".exe"], SearchOption.AllDirectories))
+            if (!FileUtils.TryGetFiles(binaryPaths, out var binaryFiles, out _, [".dll", ".exe"], ignoreFileNames, SearchOption.AllDirectories))
             {
                 errorMessage = CmdStrings.RESP_ERR_GENERIC_GETTING_BINARY_FILES;
                 return false;
             }
 
             // Check that all binary files are contained in allowed binary paths
-            var binaryFiles = files.ToArray();
             if (allowedExtensionPaths != null)
             {
                 if (binaryFiles.Any(f =>
@@ -46,7 +60,7 @@ public static bool LoadAssemblies(
             // If necessary, check that all assemblies are digitally signed
             if (!allowUnsignedAssemblies)
             {
-                foreach (var filePath in files)
+                foreach (var filePath in binaryFiles)
                 {
                     using var fs = File.OpenRead(filePath);
                     using var peReader = new PEReader(fs);
@@ -61,7 +75,7 @@ public static bool LoadAssemblies(
                     }
 
                     var publicKeyBytes = metadataReader.GetBlobBytes(assemblyPublicKeyHandle);
-                    if (publicKeyBytes == null || publicKeyBytes.Length == 0)
+                    if (publicKeyBytes.Length == 0)
                     {
                         errorMessage = CmdStrings.RESP_ERR_GENERIC_ASSEMBLY_NOT_SIGNED;
                         return false;
@@ -70,7 +84,7 @@ public static bool LoadAssemblies(
             }
 
             // Get all assemblies from binary files
-            if (!FileUtils.TryLoadAssemblies(binaryFiles, out loadedAssemblies, out _))
+            if (!FileUtils.TryLoadAssemblies(binaryFiles, out loadedAssemblies, out _) && !ignoreAssemblyLoadErrors)
             {
                 errorMessage = CmdStrings.RESP_ERR_GENERIC_LOADING_ASSEMBLIES;
                 return false;

libs/server/Resp/AdminCommands.cs

@@ -484,8 +484,29 @@ private bool NetworkModuleLoad(CustomCommandManager customCommandManager)
             for (var i = 0; i < moduleArgs.Length; i++)
                 moduleArgs[i] = parseState.GetArgSliceByRef(i + 1).ToString();
 
+            var errorMsg = ReadOnlySpan<byte>.Empty;
+
+            var binPath = Path.GetDirectoryName(modulePath);
+            var moduleFileName = Path.GetFileName(modulePath);
+
+            // Load dependencies from the module path
+            if (Directory.Exists(binPath) && !ModuleUtils.LoadAssemblies([binPath],
+                    storeWrapper.serverOptions.ExtensionBinPaths,
+                    storeWrapper.serverOptions.ExtensionAllowUnsignedAssemblies, out _, out errorMsg, [moduleFileName],
+                    SearchOption.TopDirectoryOnly, true))
+            {
+                if (!errorMsg.IsEmpty)
+                {
+                    while (!RespWriteUtils.WriteError(errorMsg, ref dcurr, dend))
+                        SendAndReset();
+                }
+
+                return true;
+            }
+
+            // Load the module path
             if (ModuleUtils.LoadAssemblies([modulePath], storeWrapper.serverOptions.ExtensionBinPaths,
-                storeWrapper.serverOptions.ExtensionAllowUnsignedAssemblies, out var loadedAssemblies, out var errorMsg))
+                storeWrapper.serverOptions.ExtensionAllowUnsignedAssemblies, out var loadedAssemblies, out errorMsg))
             {
                 Debug.Assert(loadedAssemblies != null);
                 var assembliesList = loadedAssemblies.ToList();

playground/GarnetJSON/GarnetJSON.csproj

@@ -7,12 +7,13 @@
   </PropertyGroup>
 
   <PropertyGroup>
+		<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
   </PropertyGroup>
 
-  <ItemGroup>
-    <PackageReference Include="Newtonsoft.Json" />
+	<ItemGroup>
+    <PackageReference Include="JsonPath.Net" />
   </ItemGroup>
 
   <ItemGroup>

playground/GarnetJSON/JsonCommands.cs

@@ -47,8 +47,9 @@ public override bool Reader(ReadOnlyMemory<byte> key, ref ObjectInput input, IGa
 
             var offset = 0;
             var path = CustomCommandUtils.GetNextArg(ref input, ref offset);
+            var strPath = path.IsEmpty ? "$" : Encoding.UTF8.GetString(path);
 
-            if (((JsonObject)value).TryGet(Encoding.UTF8.GetString(path), out var result, logger))
+            if (((JsonObject)value).TryGet(strPath, out var result, logger))
                 CustomCommandUtils.WriteBulkString(ref output, Encoding.UTF8.GetBytes(result));
             else
                 WriteNullBulkString(ref output);