Change the order in which events are issued (#4274)

spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/InstanceRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2022 the original author or authors.
+ * Copyright 2013-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@
 
 /**
  * @author Spencer Gibb
+ * @author Wonchul Heo
  */
 public class InstanceRegistry extends PeerAwareInstanceRegistryImpl implements ApplicationContextAware {
 
@@ -78,52 +79,65 @@ public void openForTraffic(ApplicationInfoManager applicationInfoManager, int co
 
 	@Override
 	public void register(InstanceInfo info, int leaseDuration, boolean isReplication) {
-		handleRegistration(info, leaseDuration, isReplication);
 		super.register(info, leaseDuration, isReplication);
+		handleRegistration(info, leaseDuration, isReplication);
 	}
 
 	@Override
 	public void register(final InstanceInfo info, final boolean isReplication) {
-		handleRegistration(info, resolveInstanceLeaseDuration(info), isReplication);
 		super.register(info, isReplication);
+		handleRegistration(info, resolveInstanceLeaseDuration(info), isReplication);
 	}
 
 	@Override
 	public boolean cancel(String appName, String serverId, boolean isReplication) {
-		handleCancelation(appName, serverId, isReplication);
-		return super.cancel(appName, serverId, isReplication);
+		final boolean cancelled = super.cancel(appName, serverId, isReplication);
+		if (cancelled) {
+			handleCancelation(appName, serverId, isReplication);
+		}
+		return cancelled;
 	}
 
 	@Override
 	public boolean renew(final String appName, final String serverId, boolean isReplication) {
-		log("renew " + appName + " serverId " + serverId + ", isReplication {}" + isReplication);
-		Application application = getApplication(appName);
-		if (application != null) {
-			InstanceInfo instanceInfo = application.getByInstanceId(serverId);
-			if (instanceInfo != null) {
-				publishEvent(new EurekaInstanceRenewedEvent(this, appName, serverId, instanceInfo, isReplication));
-			}
+		final boolean renewed = super.renew(appName, serverId, isReplication);
+		if (renewed) {
+			handleRenewal(appName, serverId, isReplication);
 		}
-		return super.renew(appName, serverId, isReplication);
+		return renewed;
 	}
 
 	@Override
 	protected boolean internalCancel(String appName, String id, boolean isReplication) {
-		handleCancelation(appName, id, isReplication);
-		return super.internalCancel(appName, id, isReplication);
+		final boolean cancelled = super.internalCancel(appName, id, isReplication);
+		if (cancelled) {
+			handleCancelation(appName, id, isReplication);
+		}
+		return cancelled;
 	}
 
 	private void handleCancelation(String appName, String id, boolean isReplication) {
-		log("cancel " + appName + ", serverId " + id + ", isReplication " + isReplication);
+		log("cancelled " + appName + ", serverId " + id + ", isReplication " + isReplication);
 		publishEvent(new EurekaInstanceCanceledEvent(this, appName, id, isReplication));
 	}
 
 	private void handleRegistration(InstanceInfo info, int leaseDuration, boolean isReplication) {
-		log("register " + info.getAppName() + ", vip " + info.getVIPAddress() + ", leaseDuration " + leaseDuration
+		log("registered " + info.getAppName() + ", vip " + info.getVIPAddress() + ", leaseDuration " + leaseDuration
 				+ ", isReplication " + isReplication);
 		publishEvent(new EurekaInstanceRegisteredEvent(this, info, leaseDuration, isReplication));
 	}
 
+	private void handleRenewal(final String appName, final String serverId, boolean isReplication) {
+		log("renewed " + appName + ", serverId " + serverId + ", isReplication " + isReplication);
+		final Application application = getApplication(appName);
+		if (application != null) {
+			final InstanceInfo instanceInfo = application.getByInstanceId(serverId);
+			if (instanceInfo != null) {
+				publishEvent(new EurekaInstanceRenewedEvent(this, appName, serverId, instanceInfo, isReplication));
+			}
+		}
+	}
+
 	private void log(String message) {
 		if (log.isDebugEnabled()) {
 			log.debug(message);

spring-cloud-netflix-eureka-server/src/test/java/org/springframework/cloud/netflix/eureka/server/InstanceRegistryTests.java

@@ -16,13 +16,11 @@
 
 package org.springframework.cloud.netflix.eureka.server;
 
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 
 import com.netflix.appinfo.InstanceInfo;
 import com.netflix.appinfo.LeaseInfo;
-import com.netflix.discovery.shared.Application;
 import com.netflix.eureka.registry.PeerAwareInstanceRegistry;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -41,7 +39,6 @@
 import org.springframework.context.event.SmartApplicationListener;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.doReturn;
 
 /**
  * @author Bartlomiej Slota
@@ -65,6 +62,7 @@ class InstanceRegistryTests {
 	@BeforeEach
 	void setup() {
 		this.testEvents.applicationEvents.clear();
+		this.instanceRegistry.clearRegistry();
 	}
 
 	@Autowired
@@ -103,47 +101,49 @@ void testDefaultLeaseDurationRegisterEvent() {
 
 	@Test
 	void testInternalCancel() {
+		// registering instance info
+		final InstanceInfo instanceInfo = getInstanceInfo(APP_NAME, HOST_NAME, INSTANCE_ID, PORT, null);
+		instanceRegistry.register(instanceInfo, false);
 		// calling tested method
-		instanceRegistry.internalCancel(APP_NAME, HOST_NAME, false);
+		instanceRegistry.internalCancel(APP_NAME, INSTANCE_ID, false);
 		// event of proper type is registered
-		assertThat(this.testEvents.applicationEvents.size()).isEqualTo(1);
-		assertThat(this.testEvents.applicationEvents.get(0) instanceof EurekaInstanceCanceledEvent).isTrue();
+		assertThat(this.testEvents.applicationEvents.size()).isEqualTo(2);
+		assertThat(this.testEvents.applicationEvents.get(0) instanceof EurekaInstanceRegisteredEvent).isTrue();
+		assertThat(this.testEvents.applicationEvents.get(1) instanceof EurekaInstanceCanceledEvent).isTrue();
 		// event details are correct
 		final EurekaInstanceCanceledEvent registeredEvent = (EurekaInstanceCanceledEvent) (this.testEvents.applicationEvents
-				.get(0));
+				.get(1));
 		assertThat(registeredEvent.getAppName()).isEqualTo(APP_NAME);
-		assertThat(registeredEvent.getServerId()).isEqualTo(HOST_NAME);
+		assertThat(registeredEvent.getServerId()).isEqualTo(INSTANCE_ID);
 		assertThat(registeredEvent.getSource()).isEqualTo(instanceRegistry);
 		assertThat(registeredEvent.isReplication()).isFalse();
 	}
 
 	@Test
 	void testRenew() {
-		// Creating two instances of the app
+		// registering two instances of the app
 		final InstanceInfo instanceInfo1 = getInstanceInfo(APP_NAME, HOST_NAME, INSTANCE_ID, PORT, null);
 		final InstanceInfo instanceInfo2 = getInstanceInfo(APP_NAME, HOST_NAME, "my-host-name:8009", 8009, null);
-		// creating application list with an app having two instances
-		final Application application = new Application(APP_NAME, Arrays.asList(instanceInfo1, instanceInfo2));
-		// stubbing application
-		doReturn(application).when(instanceRegistry).getApplication(APP_NAME);
+		instanceRegistry.register(instanceInfo1, false);
+		instanceRegistry.register(instanceInfo2, false);
 		// calling tested method
 		instanceRegistry.renew(APP_NAME, INSTANCE_ID, false);
 		instanceRegistry.renew(APP_NAME, "my-host-name:8009", false);
 		// event of proper type is registered
-		assertThat(this.testEvents.applicationEvents.size()).isEqualTo(2);
-		assertThat(this.testEvents.applicationEvents.get(0) instanceof EurekaInstanceRenewedEvent).isTrue();
-		assertThat(this.testEvents.applicationEvents.get(1) instanceof EurekaInstanceRenewedEvent).isTrue();
+		assertThat(this.testEvents.applicationEvents.size()).isEqualTo(4);
+		assertThat(this.testEvents.applicationEvents.get(2) instanceof EurekaInstanceRenewedEvent).isTrue();
+		assertThat(this.testEvents.applicationEvents.get(3) instanceof EurekaInstanceRenewedEvent).isTrue();
 		// event details are correct
 		final EurekaInstanceRenewedEvent event1 = (EurekaInstanceRenewedEvent) (this.testEvents.applicationEvents
-				.get(0));
+				.get(2));
 		assertThat(event1.getAppName()).isEqualTo(APP_NAME);
 		assertThat(event1.getServerId()).isEqualTo(INSTANCE_ID);
 		assertThat(event1.getSource()).isEqualTo(instanceRegistry);
 		assertThat(event1.getInstanceInfo()).isEqualTo(instanceInfo1);
 		assertThat(event1.isReplication()).isFalse();
 
 		final EurekaInstanceRenewedEvent event2 = (EurekaInstanceRenewedEvent) (this.testEvents.applicationEvents
-				.get(1));
+				.get(3));
 		assertThat(event2.getInstanceInfo()).isEqualTo(instanceInfo2);
 	}