Refactor deletion of Extension in Extension client (#2267)

#### What type of PR is this?

/kind improvement
/area core
/milestone 2.0

#### What this PR does / why we need it:

Do not delete directly when invoking ExtensionClient#delete. We just flag it by setting metadata.deletionTimestamp.

The rest should be done by garbage collector.

#### Does this PR introduce a user-facing change?

```release-note
None
```
pull/2269/head
John Niang 2022-07-21 14:29:52 +08:00 committed by GitHub
parent bb0b5b26e2
commit 50932fc2a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 17 additions and 19 deletions

View File

@ -105,8 +105,10 @@ public class DefaultExtensionClient implements ExtensionClient {
@Override @Override
public <E extends Extension> void delete(E extension) { public <E extends Extension> void delete(E extension) {
extension.getMetadata().setDeletionTimestamp(Instant.now());
var extensionStore = converter.convertTo(extension); var extensionStore = converter.convertTo(extension);
var deleteStore = storeClient.delete(extensionStore.getName(), extensionStore.getVersion()); var deleteStore = storeClient.update(extensionStore.getName(), extensionStore.getVersion(),
extensionStore.getData());
Extension deleteExt = converter.convertFrom(extension.getClass(), deleteStore); Extension deleteExt = converter.convertFrom(extension.getClass(), deleteStore);
watchers.onDelete(deleteExt); watchers.onDelete(deleteExt);
} }

View File

@ -6,7 +6,6 @@ import static org.springdoc.core.fn.builders.requestbody.Builder.requestBodyBuil
import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.enums.ParameterIn;
import java.net.URI; import java.net.URI;
import java.time.Instant;
import java.util.Objects; import java.util.Objects;
import net.bytebuddy.ByteBuddy; import net.bytebuddy.ByteBuddy;
import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.description.type.TypeDescription;
@ -299,10 +298,7 @@ public class ExtensionRouterFunctionFactory {
String name = request.pathVariable("name"); String name = request.pathVariable("name");
return getExtension(name) return getExtension(name)
.flatMap(extension -> .flatMap(extension ->
Mono.fromRunnable(() -> { Mono.fromRunnable(() -> client.delete(extension)).thenReturn(extension))
extension.getMetadata().setDeletionTimestamp(Instant.now());
client.update(extension);
}).thenReturn(extension))
.flatMap(extension -> this.getExtension(name)) .flatMap(extension -> this.getExtension(name))
.flatMap(extension -> ServerResponse .flatMap(extension -> ServerResponse
.ok() .ok()

View File

@ -29,6 +29,7 @@ import run.halo.app.extension.FakeExtension;
import run.halo.app.extension.Metadata; import run.halo.app.extension.Metadata;
import run.halo.app.extension.Scheme; import run.halo.app.extension.Scheme;
import run.halo.app.extension.SchemeManager; import run.halo.app.extension.SchemeManager;
import run.halo.app.extension.store.ExtensionStoreRepository;
@SpringBootTest @SpringBootTest
@AutoConfigureWebTestClient @AutoConfigureWebTestClient
@ -59,6 +60,11 @@ class ExtensionConfigurationTest {
schemeManager.register(FakeExtension.class); schemeManager.register(FakeExtension.class);
} }
@AfterEach
void cleanUp(@Autowired ExtensionStoreRepository repository) {
repository.deleteAll();
}
@Test @Test
@WithMockUser @WithMockUser
void shouldReturnNotFoundWhenSchemeNotRegistered() { void shouldReturnNotFoundWhenSchemeNotRegistered() {
@ -130,12 +136,6 @@ class ExtensionConfigurationTest {
.getResponseBody(); .getResponseBody();
} }
@AfterEach
void cleanUp() {
FakeExtension fakeToDelete = getFakeExtension(createdFake.getMetadata().getName());
extClient.delete(fakeToDelete);
}
@Test @Test
@WithMockUser @WithMockUser
void shouldDeleteExtensionWhenSchemeRegistered() { void shouldDeleteExtensionWhenSchemeRegistered() {

View File

@ -12,6 +12,7 @@ import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA; import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@ -340,13 +341,14 @@ class DefaultExtensionClientTest {
var fake = createFakeExtension("fake", 2L); var fake = createFakeExtension("fake", 2L);
when(converter.convertTo(any())).thenReturn( when(converter.convertTo(any())).thenReturn(
createExtensionStore("/registry/fake.halo.run/fakes/fake")); createExtensionStore("/registry/fake.halo.run/fakes/fake"));
when(storeClient.delete(any(), any())).thenReturn( when(storeClient.update(any(), any(), any())).thenReturn(
createExtensionStore("/registry/fake.halo.run/fakes/fake")); createExtensionStore("/registry/fake.halo.run/fakes/fake"));
client.delete(fake); client.delete(fake);
verify(converter, times(1)).convertTo(any()); verify(converter, times(1)).convertTo(any());
verify(storeClient, times(1)).delete(any(), any()); verify(storeClient, times(1)).update(any(), any(), any());
verify(storeClient, never()).delete(any(), any());
} }
@Nested @Nested

View File

@ -5,7 +5,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.same; import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
@ -57,7 +56,7 @@ class ExtensionDeleteHandlerTest {
.pathVariable("name", "my-fake") .pathVariable("name", "my-fake")
.body(Mono.just(unstructured)); .body(Mono.just(unstructured));
when(client.fetch(eq(FakeExtension.class), eq("my-fake"))).thenReturn(Optional.of(fake)); when(client.fetch(eq(FakeExtension.class), eq("my-fake"))).thenReturn(Optional.of(fake));
doNothing().when(client).update(any()); doNothing().when(client).delete(any());
var scheme = Scheme.buildFromType(FakeExtension.class); var scheme = Scheme.buildFromType(FakeExtension.class);
var deleteHandler = new ExtensionDeleteHandler(scheme, client); var deleteHandler = new ExtensionDeleteHandler(scheme, client);
@ -72,9 +71,8 @@ class ExtensionDeleteHandlerTest {
}) })
.verifyComplete(); .verifyComplete();
verify(client, times(2)).fetch(eq(FakeExtension.class), eq("my-fake")); verify(client, times(2)).fetch(eq(FakeExtension.class), eq("my-fake"));
verify(client, times(1)).update( verify(client, times(1)).delete(any());
argThat(fakeToDelete -> fakeToDelete.getMetadata().getDeletionTimestamp() != null)); verify(client, times(0)).update(any());
verify(client, times(0)).delete(any());
} }
@Test @Test