Merge pull request #29 from TownyAdvanced/land_validation_job_fix

Land validation job fix
TownyAdvanced · Jun 24, 2023 · 6518358 · 6518358
2 parents 2caf950 + ae20efa
commit 6518358
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 47 deletions.
diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>io.github.townyadvanced</groupId>
   <artifactId>TownyProvinces</artifactId>
-  <version>1.4.1</version>
+  <version>1.4.2</version>
   <name>townyprovinces</name> <!-- Leave lower-cased -->
 
   <properties>

diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/TownyProvinces.java b/src/main/java/io/github/townyadvanced/townyprovinces/TownyProvinces.java
@@ -30,6 +30,7 @@ public class TownyProvinces extends JavaPlugin {
 	 * to avoid concurrent modification problems
 	 */
 	public static final Object DYNMAP_DISPLAY_LOCK = new Object();
+	public static final Object LAND_VALIDATION_LOCK = new Object();
 	private static TownyProvinces plugin;
 	private static final Version requiredTownyVersion = Version.fromString("0.99.1.0");
 

diff --git a/...ithub/townyadvanced/townyprovinces/jobs/land_validation/LandValidationTaskController.java b/...ithub/townyadvanced/townyprovinces/jobs/land_validation/LandValidationTaskController.java
@@ -2,46 +2,30 @@
 
 import com.palmergames.bukkit.towny.object.Translatable;
 import io.github.townyadvanced.townyprovinces.TownyProvinces;
-import io.github.townyadvanced.townyprovinces.data.DataHandlerUtil;
-import io.github.townyadvanced.townyprovinces.data.TownyProvincesDataHolder;
 import io.github.townyadvanced.townyprovinces.messaging.Messaging;
-import io.github.townyadvanced.townyprovinces.objects.Province;
 
 public class LandValidationTaskController {
 	private static LandValidationJobStatus landValidationJobStatus;
 
 	static {
-		if(areAnyValidationsPending()) {
-			landValidationJobStatus = LandValidationJobStatus.PAUSED;
-		} else {
-			landValidationJobStatus = LandValidationJobStatus.STOPPED;
-		}
+		//Could actually be paused, so this could be misleading
+		//Todo - Maybe improve in future. You would need to do a read in a separate thread to determine if paused
+		landValidationJobStatus = LandValidationJobStatus.STOPPED;
 	}
 
 	private static LandvalidationTask landValidationTask = null;
-	public static boolean startTask() {
-		/*
-		 * If there are any requests pending, just start the job
-		 * otherwise request all provinces
-		 */
-		if(!areAnyValidationsPending()) {
-			setLandValidationRequestsForAllProvinces(true);
-		}
+	public static void startTask() {
 		landValidationTask = new LandvalidationTask();
 		landValidationTask.runTaskAsynchronously(TownyProvinces.getPlugin());
 		landValidationJobStatus = LandValidationJobStatus.STARTED;
 		Messaging.sendGlobalMessage(Translatable.of("msg_land_validation_job_started"));
-		return true;
 	}
 
 	public static void stopTask() {
 		if(landValidationTask != null) {
 			landValidationTask.cancel();
 			landValidationTask = null;
-			setLandValidationRequestsForAllProvinces(false);  //Clear all requests
 			landValidationJobStatus = LandValidationJobStatus.STOPPED;
-			TownyProvinces.info("Land Validation Task: Saving data");
-			DataHandlerUtil.saveAllData();
 			Messaging.sendGlobalMessage(Translatable.of("msg_land_validation_job_stopped"));
 		}
 	}
@@ -51,8 +35,6 @@ public static void pauseTask() {
 			landValidationTask.cancel();
 			landValidationTask = null;
 			landValidationJobStatus = LandValidationJobStatus.PAUSED;
-			TownyProvinces.info("Land Validation Task: Saving data");
-			DataHandlerUtil.saveAllData();
 			Messaging.sendGlobalMessage(Translatable.of("msg_land_validation_job_paused"));
 		}
 	}
@@ -62,15 +44,6 @@ public static void restartTask() {
 		startTask();
 	}
 
-	private static void setLandValidationRequestsForAllProvinces(boolean value) {
-		for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) {
-			if(province.isLandValidationRequested() != value) {
-				province.setLandValidationRequested(value);
-				province.saveData();
-			}
-		}
-	}
-
 	public static LandValidationJobStatus getLandValidationJobStatus() {
 		return landValidationJobStatus;
 	}
@@ -79,14 +52,6 @@ public static void setLandValidationJobStatus(LandValidationJobStatus status) {
 		landValidationJobStatus = status;
 	}
 
-	private static boolean areAnyValidationsPending() {
-		for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) {
-			if(province.isLandValidationRequested()) {
-				return true;
-			}
-		}
-		return false;
-	}
 }
 
 
diff --git a/.../java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandvalidationTask.java b/.../java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandvalidationTask.java
@@ -1,7 +1,10 @@
 package io.github.townyadvanced.townyprovinces.jobs.land_validation;
 
+import com.palmergames.bukkit.towny.object.Translatable;
 import io.github.townyadvanced.townyprovinces.TownyProvinces;
+import io.github.townyadvanced.townyprovinces.data.DataHandlerUtil;
 import io.github.townyadvanced.townyprovinces.data.TownyProvincesDataHolder;
+import io.github.townyadvanced.townyprovinces.messaging.Messaging;
 import io.github.townyadvanced.townyprovinces.objects.Province;
 import io.github.townyadvanced.townyprovinces.objects.TPCoord;
 import io.github.townyadvanced.townyprovinces.settings.TownyProvincesSettings;
@@ -18,9 +21,37 @@ public class LandvalidationTask extends BukkitRunnable {
 
 	@Override
 	public void run() {
-		//Execute the land validation job
-		TownyProvinces.info("Land Validation Job Starting.");
-		executeLandValidation();
+		TownyProvinces.info("Acquiring land validation lock.");
+		synchronized (TownyProvinces.LAND_VALIDATION_LOCK) {
+			TownyProvinces.info("Land validation lock acquired.");
+			TownyProvinces.info("Land Validation Job Starting.");
+			/*
+			 * If there are no requests pending, 
+			 * this is a fresh start, so request all provinces
+			 */
+			if(!areAnyValidationsPending()) {
+				setLandValidationRequestsForAllProvinces(true);
+			}
+			executeLandValidation();
+		}
+	}
+
+	private boolean areAnyValidationsPending() {
+		for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) {
+			if(province.isLandValidationRequested()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private void setLandValidationRequestsForAllProvinces(boolean value) {
+		for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) {
+			if(province.isLandValidationRequested() != value) {
+				province.setLandValidationRequested(value);
+				province.saveData();
+			}
+		}
 	}
 
 	/**
@@ -59,12 +90,22 @@ private void executeLandValidation() {
 			LandValidationJobStatus landValidationJobStatus = LandValidationTaskController.getLandValidationJobStatus();
 			switch (landValidationJobStatus) {
 				case STOP_REQUESTED:
+					TownyProvinces.info("Land Validation Task: Clearing all validation requests");
+					setLandValidationRequestsForAllProvinces(false);  //Clear all requests
+					TownyProvinces.info("Land Validation Task: Saving data");
+					DataHandlerUtil.saveAllData();
 					LandValidationTaskController.stopTask();
 					return;
 				case PAUSE_REQUESTED:
+					TownyProvinces.info("Land Validation Task: Saving data");
+					DataHandlerUtil.saveAllData();
 					LandValidationTaskController.pauseTask();
 					return;
 				case RESTART_REQUESTED:
+					TownyProvinces.info("Land Validation Task: Clearing all validation requests");
+					setLandValidationRequestsForAllProvinces(false);  //Clear all requests
+					TownyProvinces.info("Land Validation Task: Saving data");
+					DataHandlerUtil.saveAllData();
 					LandValidationTaskController.restartTask();
 					return;
 			}

diff --git a/...io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java b/...io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java
@@ -42,10 +42,14 @@ public RegenerateRegionTask(String regionName) {
 	public void run() {
 		try {
 			TownyProvinces.info("Regeneration Job Started");
-			TownyProvinces.info("Regeneration Job: Acquiring dynmap display lock");
-			synchronized (TownyProvinces.DYNMAP_DISPLAY_LOCK) {
-				TownyProvinces.info("Regeneration Job: Dynmap display lock acquired");
-				executeRegionRegenerationJob();
+			TownyProvinces.info("Regeneration Job: Acquiring land validation lock");
+			synchronized (TownyProvinces.LAND_VALIDATION_LOCK) {
+				TownyProvinces.info("Regeneration Job: Land Validation lock acquired");
+				TownyProvinces.info("Regeneration Job: Acquiring dynmap display lock");
+				synchronized (TownyProvinces.DYNMAP_DISPLAY_LOCK) {
+					TownyProvinces.info("Regeneration Job: Dynmap display lock acquired");
+					executeRegionRegenerationJob();
+				}
 			}
 		} finally {
 			TownyProvinces.info("Regeneration Job: Dynmap display lock released");