feature(core): Add wait handler structure #19

Fyusel · 2025-09-05T06:35:46Z

Description

relates to STACKITSDK-232

Checklist

core/src/main/java/cloud/stackit/sdk/core/oapierror/GenericOpenAPIError.java

services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/Wait.java

services/resourcemanager/src/test/java/cloud/stackit/sdk/resourcemanager/WaitTest.java

services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/Wait.java

CONTRIBUTION.md

core/src/test/java/cloud/stackit/sdk/core/wait/AsyncWaitHandlerTest.java

Signed-off-by: Alexander Dahmen <[email protected]>

rubenhoenle · 2025-09-12T09:12:00Z

core/src/main/java/cloud/stackit/sdk/core/oapierror/GenericOpenAPIError.java

+	// the response Body
+	public static int ApiErrorMaxCharacterLimit = 500;
+
+	private int statusCode;


Suggested change

private int statusCode;

private final int statusCode;

could be final

rubenhoenle · 2025-09-12T09:12:19Z

core/src/main/java/cloud/stackit/sdk/core/oapierror/GenericOpenAPIError.java

+
+	private int statusCode;
+	private byte[] body;
+	private String errorMessage;


Suggested change

private String errorMessage;

private final String errorMessage;

same here

rubenhoenle · 2025-09-12T09:13:00Z

...esourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/ResourcemanagerWait.java

+
+public class ResourcemanagerWait {
+	/**
+	 * createProjectWaitHandler will wait for project creation. Uses the deault values for


Suggested change

* createProjectWaitHandler will wait for project creation. Uses the deault values for

* createProjectWaitHandler will wait for project creation. Uses the default values for

rubenhoenle · 2025-09-12T09:15:10Z

...esourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/ResourcemanagerWait.java

+				() -> {
+					try {
+						GetProjectResponse p = apiClient.getProject(containerId, false);
+						if (p.getContainerId() == null || p.getLifecycleState() == null) {


This condition will always be false I guess. Both getContainerId() and getLifecycleState() are annotated with Nonnull.

stackit-sdk-java/services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/model/GetProjectResponse.java

Lines 107 to 110 in 44d23b5

@javax.annotation.Nonnull

public String getContainerId() {

return containerId;

}

stackit-sdk-java/services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/model/GetProjectResponse.java

Lines 175 to 178 in 44d23b5

@javax.annotation.Nonnull

public LifecycleState getLifecycleState() {

return lifecycleState;

}

rubenhoenle · 2025-09-12T09:15:42Z

...esourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/ResourcemanagerWait.java

+				() -> {
+					try {
+						GetProjectResponse p = apiClient.getProject(containerId, false);
+						if (p.getContainerId() == null || p.getLifecycleState() == null) {


Same here, always false.

rubenhoenle · 2025-09-12T09:20:30Z

.../src/test/java/cloud/stackit/sdk/resourcemanager/ResourcemanagerWaitTestmanagerWaitTest.java

+		handler.setThrottle(10, TimeUnit.MILLISECONDS);
+		handler.setTimeout(500, TimeUnit.MILLISECONDS);
+
+		assertThrows(Exception.class, () -> handler.waitWithContext(), handler.TimoutErrorMessage);


Suggested change

assertThrows(Exception.class, () -> handler.waitWithContext(), handler.TimoutErrorMessage);

assertThrows(Exception.class, handler::waitWithContext, handler.TimoutErrorMessage);

Appears multiple times within this class, please check all places

rubenhoenle · 2025-09-12T09:22:29Z

.../src/test/java/cloud/stackit/sdk/resourcemanager/ResourcemanagerWaitTestmanagerWaitTest.java

+	void testCreateProjectNonOpenAPIError() throws Exception {
+		// Return containerId == null and LifecycleState == CREATING
+		GetProjectResponse creatingResponse = new GetProjectResponse();
+		creatingResponse.setContainerId(null);


Parameter is annotated with notnull.

stackit-sdk-java/services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/model/GetProjectResponse.java

Lines 112 to 114 in 44d23b5

public void setContainerId(@javax.annotation.Nonnull String containerId) {

this.containerId = containerId;

}

rubenhoenle · 2025-09-12T09:22:34Z

.../src/test/java/cloud/stackit/sdk/resourcemanager/ResourcemanagerWaitTestmanagerWaitTest.java

+		// Return containerId == null and LifecycleState == CREATING
+		GetProjectResponse creatingResponse = new GetProjectResponse();
+		creatingResponse.setContainerId(null);
+		creatingResponse.setLifecycleState(null);


rubenhoenle · 2025-09-12T09:23:03Z

.../src/test/java/cloud/stackit/sdk/resourcemanager/ResourcemanagerWaitTestmanagerWaitTest.java

+		activeResponse.setLifecycleState(LifecycleState.ACTIVE);
+
+		GetProjectResponse deletingResponse = new GetProjectResponse();
+		deletingResponse.setContainerId(null);


Same here, param is annotated with notnull.

stackit-sdk-java/services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/model/GetProjectResponse.java

Lines 112 to 114 in 44d23b5

public void setContainerId(@javax.annotation.Nonnull String containerId) {

this.containerId = containerId;

}

rubenhoenle · 2025-09-12T09:26:55Z

core/src/main/java/cloud/stackit/sdk/core/wait/AsyncActionHandler.java

+		// Wait some seconds for the API to process the request
+		if (sleepBeforeWaitMillis > 0) {
+			try {
+				Thread.sleep(sleepBeforeWaitMillis);


Not sure if it's that nice to set a whole thread asleep.

On the other side we're limited to Java 8 features 😄 Did you do some research if there are better alternatives?

Will throw in e.g. CompletableFutures here, it was introduced with Java 8.

rubenhoenle · 2025-09-12T09:36:55Z

core/src/main/java/cloud/stackit/sdk/core/wait/AsyncActionHandler.java

+	 * @return
+	 * @throws Exception
+	 */
+	public T waitWithContext() throws Exception {


Suggested change

public T waitWithContext() throws Exception {

public T waitWithContext() throws InterruptedException, IllegalArgumentException, TimeoutException {

Throwing Exception is a little bit... to much here. Users of the SDK won't have the chance to handle this properly.

rubenhoenle · 2025-09-12T09:39:35Z

core/src/main/java/cloud/stackit/sdk/core/oapierror/GenericOpenAPIError.java

+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+
+public class GenericOpenAPIError extends ApiException {


Suggested change

public class GenericOpenAPIError extends ApiException {

public class GenericOpenAPIException extends ApiException {

Would keep the "Exception" in the name here instead of "Error". "Error" is a golang thing, Java has Exceptions.

rubenhoenle · 2025-09-12T09:41:59Z

core/src/main/java/cloud/stackit/sdk/core/wait/AsyncActionHandler.java

+		throw new Exception(TimoutErrorMessage);
+	}
+
+	private ErrorResult handleError(int retryTempErrorCounter, Exception err) {


Suggested change

private ErrorResult handleError(int retryTempErrorCounter, Exception err) {

private ErrorResult handleError(int retryTempErrorCounter, Exception exception) {

rubenhoenle · 2025-09-12T09:42:10Z

core/src/main/java/cloud/stackit/sdk/core/wait/AsyncActionHandler.java

+		throw new Exception(TimoutErrorMessage);
+	}
+
+	private ErrorResult handleError(int retryTempErrorCounter, Exception err) {


Suggested change

private ErrorResult handleError(int retryTempErrorCounter, Exception err) {

private ErrorResult handleException(int retryTempErrorCounter, Exception err) {

Overall this feels a little bit to me like taking the Golang waiter implementation and strictly rewriting it in Java. Java and Go have a completely different approach of error raising and handling.

Signed-off-by: Alexander Dahmen <[email protected]>

rubenhoenle · 2025-09-12T10:32:14Z

...esourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/ResourcemanagerWait.java

+												+ p.getLifecycleState().getValue()
+												+ "'"));
+					} catch (ApiException e) {
+						return new AsyncActionResult<>(false, null, e);


Maybe I'm wrong here but my understanding is that in Java you don't return exceptions, you throw and catch.

rubenhoenle · 2025-09-12T10:35:00Z

...esourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/ResourcemanagerWait.java

+						return new AsyncActionResult<>(false, null, null);
+
+					} catch (ApiException e) {
+						GenericOpenAPIException oapiErr = new GenericOpenAPIException(e);


This catch block seems generic to me and could apply to most of our deletion wait handlers. Maybe we can have some general approach for different wait handler types (Interfaces, abstract classes)?

Signed-off-by: Alexander Dahmen <[email protected]>

Fyusel requested a review from a team as a code owner September 5, 2025 06:35

rubenhoenle reviewed Sep 5, 2025

View reviewed changes

core/src/main/java/cloud/stackit/sdk/core/oapierror/GenericOpenAPIError.java Outdated Show resolved Hide resolved

rubenhoenle reviewed Sep 5, 2025

View reviewed changes

services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/Wait.java Outdated Show resolved Hide resolved

rubenhoenle reviewed Sep 5, 2025

View reviewed changes

services/resourcemanager/src/test/java/cloud/stackit/sdk/resourcemanager/WaitTest.java Outdated Show resolved Hide resolved

rubenhoenle reviewed Sep 5, 2025

View reviewed changes

services/resourcemanager/src/main/java/cloud/stackit/sdk/resourcemanager/wait/Wait.java Outdated Show resolved Hide resolved

Fyusel force-pushed the ft/java-wait-handler branch 2 times, most recently from 060607e to b4e7330 Compare September 9, 2025 09:13

rubenhoenle reviewed Sep 9, 2025

View reviewed changes

CONTRIBUTION.md Outdated Show resolved Hide resolved

CONTRIBUTION.md Outdated Show resolved Hide resolved

core/src/test/java/cloud/stackit/sdk/core/wait/AsyncWaitHandlerTest.java Outdated Show resolved Hide resolved

Fyusel force-pushed the ft/java-wait-handler branch 2 times, most recently from 12a9371 to 49fb05c Compare September 10, 2025 06:29

feature(core): Add wait handler structure

5d4d5ff

Signed-off-by: Alexander Dahmen <[email protected]>

rubenhoenle reviewed Sep 12, 2025

View reviewed changes

Fyusel force-pushed the ft/java-wait-handler branch from 49fb05c to 59254e9 Compare September 12, 2025 10:01

fix(wait): linter issues

3a0eea7

Signed-off-by: Alexander Dahmen <[email protected]>

Fyusel force-pushed the ft/java-wait-handler branch from 59254e9 to 3a0eea7 Compare September 12, 2025 10:03

rubenhoenle reviewed Sep 12, 2025

View reviewed changes

chore(wait): Rework wait handler to use CompletableFuture

180f373

Signed-off-by: Alexander Dahmen <[email protected]>

	private String errorMessage;
	private final String errorMessage;

	* createProjectWaitHandler will wait for project creation. Uses the deault values for
	* createProjectWaitHandler will wait for project creation. Uses the default values for

	@javax.annotation.Nonnull
	public String getContainerId() {
	return containerId;
	}

	@javax.annotation.Nonnull
	public LifecycleState getLifecycleState() {
	return lifecycleState;
	}

	assertThrows(Exception.class, () -> handler.waitWithContext(), handler.TimoutErrorMessage);
	assertThrows(Exception.class, handler::waitWithContext, handler.TimoutErrorMessage);

	public void setContainerId(@javax.annotation.Nonnull String containerId) {
	this.containerId = containerId;
	}

	public T waitWithContext() throws Exception {
	public T waitWithContext() throws InterruptedException, IllegalArgumentException, TimeoutException {

	public class GenericOpenAPIError extends ApiException {
	public class GenericOpenAPIException extends ApiException {

	private ErrorResult handleError(int retryTempErrorCounter, Exception err) {
	private ErrorResult handleError(int retryTempErrorCounter, Exception exception) {

	private ErrorResult handleError(int retryTempErrorCounter, Exception err) {
	private ErrorResult handleException(int retryTempErrorCounter, Exception err) {

feature(core): Add wait handler structure #19

Are you sure you want to change the base?

feature(core): Add wait handler structure #19

Uh oh!

Conversation

Fyusel commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rubenhoenle Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Fyusel commented Sep 5, 2025 •

edited

Loading

rubenhoenle Sep 12, 2025 •

edited

Loading