RavenDB-27141 Reject two licence sources and fix from_external_server

ravendb_embedded/options.py

@@ -103,8 +103,14 @@ def INSTANCE(cls):
 
     @classmethod
     def from_external_server(cls, server_location: str) -> ServerOptions:
+        warnings.warn(
+            "ServerOptions.from_external_server() is deprecated; construct ServerOptions() and "
+            "call with_external_server(), which also runs a server directory in place.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         instance = cls()
-        instance.provider = ExternalServerProvider(server_location)
+        instance.with_external_server(server_location)
         return instance
 
     def secured(

ravendb_embedded/raven_server_runner.py

@@ -87,9 +87,12 @@ def run(options: ServerOptions) -> subprocess.Popen:
             f"--Logs.Path={options.logs_path}",
         ]
 
+        if options.licensing.license is not None and options.licensing.license_path is not None:
+            raise ValueError("Only one of the licence options 'license' or 'license_path' can be set, not both.")
+
         if options.licensing.license is not None:
             command_line_args.append(f"--License={options.licensing.license}")
-        if options.licensing.license_path is not None:
+        elif options.licensing.license_path is not None:
             command_line_args.append(f"--License.Path={options.licensing.license_path}")
 
         if options.security:

tests/test_licensing.py

@@ -11,7 +11,8 @@ class TestLicensing(TestCase):
     def test_eula_acceptance_remains_enabled_by_default(self):
         self.assertTrue(ServerOptions().accept_eula)
 
-    def test_server_receives_all_licensing_options(self):
+    def _arguments_for(self, configure):
+        """Start a fake Raven.Server that dumps its argv, and return what it was given."""
         with tempfile.TemporaryDirectory() as directory:
             server_directory = Path(directory, "Server")
             server_directory.mkdir()
@@ -32,21 +33,47 @@ def test_server_receives_all_licensing_options(self):
             options.framework_version = ""
             options.data_directory = str(Path(directory, "data"))
             options.logs_path = str(Path(directory, "logs"))
+            configure(options, directory)
+
+            with EmbeddedServer() as server:
+                server.start_server(options)
+
+            return json.loads(arguments_file.read_text(encoding="utf-8")), options
+
+    def test_server_receives_all_licensing_options(self):
+        def configure(options, directory):
             options.licensing.license = '{"Id":"test-license"}'
-            options.licensing.license_path = str(Path(directory, "license.json"))
             options.licensing.disable_auto_update = True
             options.licensing.disable_auto_update_from_api = True
             options.licensing.disable_license_support_check = False
             options.licensing.throw_on_invalid_or_missing_license = True
 
+        arguments, _ = self._arguments_for(configure)
+
+        self.assertIn("--License.Eula.Accepted=true", arguments)
+        self.assertIn("--License.DisableAutoUpdate=true", arguments)
+        self.assertIn("--License.DisableAutoUpdateFromApi=true", arguments)
+        self.assertIn("--License.DisableLicenseSupportCheck=false", arguments)
+        self.assertIn("--License.ThrowOnInvalidOrMissingLicense=true", arguments)
+        self.assertIn('--License={"Id":"test-license"}', arguments)
+        self.assertEqual([], [argument for argument in arguments if argument.startswith("--License.Path=")])
+
+    def test_a_licence_path_is_passed_when_there_is_no_inline_licence(self):
+        def configure(options, directory):
+            options.licensing.license_path = str(Path(directory, "license.json"))
+
+        arguments, options = self._arguments_for(configure)
+
+        self.assertIn(f"--License.Path={options.licensing.license_path}", arguments)
+        self.assertEqual([], [argument for argument in arguments if argument.startswith("--License=")])
+
+    def test_setting_both_licence_sources_is_rejected(self):
+        # C# raises here too (RavenServerRunner.cs:37-39). Emitting both let the server pick one
+        # and the mistake stayed invisible.
+        options = ServerOptions()
+        options.licensing.license = '{"Id":"test-license"}'
+        options.licensing.license_path = "license.json"
+
+        with self.assertRaisesRegex(ValueError, "[Oo]nly one of the licence options"):
             with EmbeddedServer() as server:
                 server.start_server(options)
-
-            arguments = json.loads(arguments_file.read_text(encoding="utf-8"))
-            self.assertIn("--License.Eula.Accepted=true", arguments)
-            self.assertIn("--License.DisableAutoUpdate=true", arguments)
-            self.assertIn("--License.DisableAutoUpdateFromApi=true", arguments)
-            self.assertIn("--License.DisableLicenseSupportCheck=false", arguments)
-            self.assertIn("--License.ThrowOnInvalidOrMissingLicense=true", arguments)
-            self.assertIn('--License={"Id":"test-license"}', arguments)
-            self.assertIn(f"--License.Path={options.licensing.license_path}", arguments)

tests/test_options.py

@@ -1,3 +1,5 @@
+import tempfile
+import warnings
 from pathlib import Path
 from unittest import TestCase
 
@@ -28,3 +30,16 @@ def test_instance_is_a_deprecated_constructor_alias(self):
             options = ServerOptions.INSTANCE()
 
         self.assertIsInstance(options, ServerOptions)
+
+    def test_from_external_server_is_deprecated_and_runs_a_directory_in_place(self):
+        with tempfile.TemporaryDirectory() as directory:
+            Path(directory, "Raven.Server.dll").write_text("", encoding="utf-8")
+            with warnings.catch_warnings(record=True) as caught:
+                warnings.simplefilter("always")
+                options = ServerOptions.from_external_server(directory)
+
+        self.assertIs(DeprecationWarning, caught[0].category)
+        self.assertIn("with_external_server", str(caught[0].message))
+        # The classmethod used to skip this, so a server directory was copied instead of run.
+        self.assertEqual(directory, options.target_server_location)
+        self.assertFalse(options.clear_target_server_location)