diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 690728d1ea..75f70f538d 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -91,16 +91,66 @@ jobs: path: "**/build/reports" - test: - name: "Test" - timeout-minutes: 120 + test_registry_unit: + name: "Test Registry Unit" needs: build - if: | - (github.event.pull_request.draft == false) - && (needs.check.outputs.testing_needed == 'true') + runs-on: ubuntu-latest + steps: + - name: "Checkout Branch" + uses: actions/checkout@v4 + + - name: "Setup Java" + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: 21 + cache: 'gradle' + - name: Setup Gradle cache + uses: actions/cache@v4 + with: + path: ~/.gradle + key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} + + - name: "Prepare libs for tests" + run: ./prepare.sh + + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.registry.unit.test:check + + + test_registry_message: + name: "Test Registry Message" + needs: build runs-on: ubuntu-latest + steps: + - name: "Checkout Branch" + uses: actions/checkout@v4 + + - name: "Setup Java" + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: 21 + cache: 'gradle' + - name: Setup Gradle cache + uses: actions/cache@v4 + with: + path: ~/.gradle + key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} + + - name: "Prepare libs for tests" + run: ./prepare.sh + + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.registry.message.test:check + + + test_registry_class: + name: "Test Registry Class" + needs: build + runs-on: ubuntu-latest steps: - name: "Checkout Branch" uses: actions/checkout@v4 @@ -118,16 +168,120 @@ jobs: path: ~/.gradle key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} - - name: "prepare libs for tests" + - name: "Prepare libs for tests" run: ./prepare.sh - - name: "test backend" - run: ./gradlew --build-cache check + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.registry.class.test:check - - name: Upload test reports - uses: actions/upload-artifact@v4 - if: ${{ failure() || contains(github.event.pull_request.labels.*.name, 'force reports') }} + + test_authentication: + name: "Test Authentication" + needs: build + runs-on: ubuntu-latest + steps: + - name: "Checkout Branch" + uses: actions/checkout@v4 + + - name: "Setup Java" + uses: actions/setup-java@v4 with: - name: Test Reports - path: "**/build/reports" + distribution: 'temurin' + java-version: 21 + cache: 'gradle' + + - name: Setup Gradle cache + uses: actions/cache@v4 + with: + path: ~/.gradle + key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} + + - name: "Prepare libs for tests" + run: ./prepare.sh + + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.authentication.test:check + + + test_dal: + name: "Test DAL" + needs: build + runs-on: ubuntu-latest + steps: + - name: "Checkout Branch" + uses: actions/checkout@v4 + + - name: "Setup Java" + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: 21 + cache: 'gradle' + + - name: Setup Gradle cache + uses: actions/cache@v4 + with: + path: ~/.gradle + key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} + + - name: "Prepare libs for tests" + run: ./prepare.sh + + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.dal.test:check + + + test_app: + name: "Test App" + needs: build + runs-on: ubuntu-latest + steps: + - name: "Checkout Branch" + uses: actions/checkout@v4 + + - name: "Setup Java" + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: 21 + cache: 'gradle' + + - name: Setup Gradle cache + uses: actions/cache@v4 + with: + path: ~/.gradle + key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} + + - name: "Prepare libs for tests" + run: ./prepare.sh + + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.app.test:check + + + test_app_preset: + name: "Test App Preset" + needs: build + runs-on: ubuntu-latest + steps: + - name: "Checkout Branch" + uses: actions/checkout@v4 + + - name: "Setup Java" + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: 21 + cache: 'gradle' + + - name: Setup Gradle cache + uses: actions/cache@v4 + with: + path: ~/.gradle + key: ${{ runner.os }}-gradle-${{ hashFiles('versions.properties') }}-${{ github.sha }} + + - name: "Prepare libs for tests" + run: ./prepare.sh + - name: "Run Tests" + run: ./gradlew --build-cache --continue --rerun-tasks :bco.app.preset:check diff --git a/.github/workflows/deploy-docker.yml b/.github/workflows/deploy-docker.yml index f87104fd2a..6182b3226e 100644 --- a/.github/workflows/deploy-docker.yml +++ b/.github/workflows/deploy-docker.yml @@ -16,7 +16,7 @@ on: jobs: docker: - # if: ${{ github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'preview') }} + if: ${{ github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'preview') }} runs-on: ubuntu-latest outputs: bco-tags: ${{ steps.bco-meta.outputs.tags }} diff --git a/.github/workflows/update-addon-version.yaml b/.github/workflows/update-addon-version.yaml index 988d4f2fb2..129d0be867 100644 --- a/.github/workflows/update-addon-version.yaml +++ b/.github/workflows/update-addon-version.yaml @@ -95,7 +95,7 @@ jobs: git config user.name "$GIT_USERNAME" git config user.email "$GIT_EMAIL" git add config.yaml - git commit -m "Update add-on $ADDONS_DIR version to $VERSION" || { + git commit -m "Update add-on $ADDON_DIR version to $VERSION" || { echo "No changes to commit" exit 0 } diff --git a/buildSrc/src/main/kotlin/org.openbase.bco.gradle.kts b/buildSrc/src/main/kotlin/org.openbase.bco.gradle.kts index fd3ae12c73..028366f412 100644 --- a/buildSrc/src/main/kotlin/org.openbase.bco.gradle.kts +++ b/buildSrc/src/main/kotlin/org.openbase.bco.gradle.kts @@ -53,7 +53,6 @@ tasks.withType { logging.captureStandardOutput(LogLevel.WARN) maxHeapSize = "7G" failFast = false - setForkEvery(1) } publishing { diff --git a/lib/jul b/lib/jul index 9d9befebe1..e5d31bf136 160000 --- a/lib/jul +++ b/lib/jul @@ -1 +1 @@ -Subproject commit 9d9befebe197fd170f73518483ce213bef661653 +Subproject commit e5d31bf1368a1aec5b6f00a3b4b70b30609dac19 diff --git a/module/app/test/src/main/java/org/openbase/app/test/agent/BCOAppTest.java b/module/app/test/src/main/java/org/openbase/app/test/agent/BCOAppTest.java index 2fd9c73e49..7968737e70 100644 --- a/module/app/test/src/main/java/org/openbase/app/test/agent/BCOAppTest.java +++ b/module/app/test/src/main/java/org/openbase/app/test/agent/BCOAppTest.java @@ -101,6 +101,9 @@ public static void tearDownBCOApp() throws Throwable { if (deviceManagerLauncher != null) { deviceManagerLauncher.shutdown(); } + if (messageManagerLauncher != null) { + messageManagerLauncher.shutdown(); + } LOGGER.info("App tests finished!"); } catch (Throwable ex) { throw ExceptionPrinter.printHistoryAndReturnThrowable(ex, LOGGER); diff --git a/module/authentication/lib/src/main/java/org/openbase/bco/authentication/lib/future/AuthenticationFutureList.kt b/module/authentication/lib/src/main/java/org/openbase/bco/authentication/lib/future/AuthenticationFutureList.kt index b9ac4c2a55..8f9ee7db34 100644 --- a/module/authentication/lib/src/main/java/org/openbase/bco/authentication/lib/future/AuthenticationFutureList.kt +++ b/module/authentication/lib/src/main/java/org/openbase/bco/authentication/lib/future/AuthenticationFutureList.kt @@ -81,4 +81,13 @@ object AuthenticationFutureList { notificationCondition.await() } } + + fun reset() { + synchronized(authenticatedFuturesLock) { + synchronized(incomingFuturesLock) { + incomingFutures.clear() + authenticatedFutures.clear() + } + } + } } diff --git a/module/authentication/test/build.gradle.kts b/module/authentication/test/build.gradle.kts index 75188b4844..eedc92c474 100644 --- a/module/authentication/test/build.gradle.kts +++ b/module/authentication/test/build.gradle.kts @@ -2,10 +2,6 @@ plugins { id("org.openbase.bco") } -configurations { - -} - dependencies { api(project(":bco.authentication.core")) api(project(":bco.authentication.lib")) diff --git a/module/authentication/test/src/test/java/org/openbase/bco/authentication/test/AuthenticationFutureListTest.kt b/module/authentication/test/src/test/java/org/openbase/bco/authentication/test/AuthenticationFutureListTest.kt index b7781c6a0c..82917ab05c 100644 --- a/module/authentication/test/src/test/java/org/openbase/bco/authentication/test/AuthenticationFutureListTest.kt +++ b/module/authentication/test/src/test/java/org/openbase/bco/authentication/test/AuthenticationFutureListTest.kt @@ -45,6 +45,7 @@ class AuthenticationFutureListTest { @Timeout(5) @Test fun testScheduledTask() { + AuthenticationFutureList.reset() val lock = ReentrantLock() val condition: Condition = lock.newCondition() diff --git a/module/dal/control/src/main/java/org/openbase/bco/dal/control/layer/unit/AbstractUnitController.java b/module/dal/control/src/main/java/org/openbase/bco/dal/control/layer/unit/AbstractUnitController.java index dfb263371e..0d6313a538 100644 --- a/module/dal/control/src/main/java/org/openbase/bco/dal/control/layer/unit/AbstractUnitController.java +++ b/module/dal/control/src/main/java/org/openbase/bco/dal/control/layer/unit/AbstractUnitController.java @@ -157,6 +157,8 @@ public abstract class AbstractUnitController scheduledActionList; private final Timeout scheduleTimeout; private boolean infrastructure = false; + private String cachedId; + private String cachedLabel; public AbstractUnitController(final DB builder) throws InstantiationException { super(builder); @@ -377,6 +379,10 @@ public UnitConfig applyConfigUpdate(final UnitConfig config) throws CouldNotPerf logger.trace("Unit config change check failed because config is not available yet."); } + // clear caches + cachedId = null; + cachedLabel = null; + try { classDescription = getClass().getSimpleName() + "[" + config.getUnitType() + "[" + LabelProcessor.getBestMatch(config.getLabel()) + "]]"; } catch (NullPointerException | NotAvailableException ex) { @@ -439,6 +445,9 @@ public UnitConfig applyConfigUpdate(final UnitConfig config) throws CouldNotPerf @Override public final String getId() throws NotAvailableException { + if (cachedId != null) { + return cachedId; + } try { UnitConfig tmpConfig = getConfig(); if (!tmpConfig.hasId()) { @@ -449,7 +458,8 @@ public final String getId() throws NotAvailableException { throw new InvalidStateException("unitconfig.id is empty"); } - return tmpConfig.getId(); + cachedId = tmpConfig.getId(); + return cachedId; } catch (CouldNotPerformException ex) { throw new NotAvailableException("Unit", "id", ex); } @@ -457,6 +467,9 @@ public final String getId() throws NotAvailableException { @Override public String getLabel() throws NotAvailableException { + if (cachedLabel != null) { + return cachedLabel; + } try { UnitConfig tmpConfig = getConfig(); if (!tmpConfig.hasLabel()) { @@ -466,7 +479,8 @@ public String getLabel() throws NotAvailableException { if (LabelProcessor.isEmpty(tmpConfig.getLabel())) { throw new InvalidStateException("unitconfig.label is empty"); } - return LabelProcessor.getBestMatch(getConfig().getLabel()); + cachedLabel = LabelProcessor.getBestMatch(getConfig().getLabel()); + return cachedLabel; } catch (CouldNotPerformException ex) { throw new NotAvailableException("Unit", "label", ex); } diff --git a/module/dal/control/src/main/java/org/openbase/bco/dal/control/message/MessageManager.kt b/module/dal/control/src/main/java/org/openbase/bco/dal/control/message/MessageManager.kt index b0cd960f05..eed9c963f6 100644 --- a/module/dal/control/src/main/java/org/openbase/bco/dal/control/message/MessageManager.kt +++ b/module/dal/control/src/main/java/org/openbase/bco/dal/control/message/MessageManager.kt @@ -41,6 +41,7 @@ class MessageManager : Launchable, VoidInitializable { fun removeOutdatedMessages(auth: AuthToken? = null) { logger.trace("removeOutdatedMessages") Registries.getMessageRegistry().userMessages + .toList() .filterNot { message -> message.conditionList.any { condition -> Units.getUnit(condition.unitId, true).let { unit -> diff --git a/module/dal/lib/src/main/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderService.java b/module/dal/lib/src/main/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderService.java index 792fa1832b..bb9b9052c3 100644 --- a/module/dal/lib/src/main/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderService.java +++ b/module/dal/lib/src/main/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderService.java @@ -22,9 +22,9 @@ * #L% */ +import org.jetbrains.annotations.NotNull; import org.openbase.bco.dal.lib.layer.service.operation.OperationService; import org.openbase.jul.annotation.RPCMethod; -import org.openbase.jul.exception.CouldNotPerformException; import org.openbase.jul.exception.CouldNotTransformException; import org.openbase.jul.exception.NotAvailableException; import org.openbase.jul.exception.VerificationFailedException; @@ -38,6 +38,9 @@ import org.openbase.type.vision.RGBColorType.RGBColor; import org.slf4j.LoggerFactory; +import java.util.function.DoubleUnaryOperator; +import java.util.function.BiPredicate; + import static org.openbase.type.domotic.service.ServiceTemplateType.ServiceTemplate.ServiceType.COLOR_STATE_SERVICE; /** @@ -164,26 +167,98 @@ static Boolean isCompatible(final ColorState colorState, final PowerState powerS } static Boolean equalServiceStates(final ColorState colorStateA, final ColorState colorStateB) { - //TODO: explain this (required because of openhab) and put margins into constants final HSBColor hsbColorA = colorStateA.getColor().getHsbColor(); final HSBColor hsbColorB = colorStateB.getColor().getHsbColor(); + // Helper: compare hues with wrap-around (0 == 360) + // margin in degrees + final double HUE_MARGIN = 1.0; + final double SATURATION_MARGIN = 0.01; + final double BRIGHTNESS_MARGIN = 0.01; + + // normalize angle to [0,360) + DoubleUnaryOperator normalize = (v) -> { + double r = v % 360.0; + if (r < 0) r += 360.0; + return r; + }; - boolean hueEquals = true; - boolean saturationEquals = true; - boolean brightnessEquals = true; + BiPredicate hueEqualsWithWrap = (ha, hb) -> { + double aNorm = normalize.applyAsDouble(ha); + double bNorm = normalize.applyAsDouble(hb); + double diff = Math.abs(aNorm - bNorm); + if (diff > 180.0) { + diff = 360.0 - diff; // shortest distance on circle + } + return diff <= HUE_MARGIN; + }; - if(hsbColorA.hasHue() && hsbColorB.hasHue()) { - hueEquals = OperationService.equals(hsbColorA.getHue(), hsbColorB.getHue(), 1.0); + boolean hueEquals; + if (hsbColorA.hasHue() && hsbColorB.hasHue()) { + hueEquals = hueEqualsWithWrap.test(hsbColorA.getHue(), hsbColorB.getHue()); + } else if (!hsbColorA.hasHue() && !hsbColorB.hasHue()) { + // both undefined -> treat as equal + hueEquals = true; + } else { + // one missing: if the present color is 'neutral' (saturation == 0 or brightness == 0 or both undefined) the hue is irrelevant + final HSBColor present = hsbColorA.hasHue() ? hsbColorA : hsbColorB; + boolean presentIsNeutral = false; + // If both saturation and brightness are missing, treat as neutral (no chroma information) + if (!present.hasSaturation() && !present.hasBrightness()) { + presentIsNeutral = true; + } + // If saturation is present and effectively 0 -> neutral + if (!presentIsNeutral && present.hasSaturation()) { + presentIsNeutral = OperationService.equals(present.getSaturation(), 0d, SATURATION_MARGIN); + } + // If not neutral yet and brightness is present and effectively 0 -> neutral + if (!presentIsNeutral && present.hasBrightness()) { + presentIsNeutral = OperationService.equals(present.getBrightness(), 0d, BRIGHTNESS_MARGIN); + } + hueEquals = presentIsNeutral; } - if(hsbColorA.hasSaturation() && hsbColorB.hasSaturation()) { - saturationEquals = OperationService.equals(hsbColorA.getSaturation(), hsbColorB.getSaturation(), 0.01); + boolean saturationEquals; + if (hsbColorA.hasSaturation() && hsbColorB.hasSaturation()) { + saturationEquals = OperationService.equals(hsbColorA.getSaturation(), hsbColorB.getSaturation(), SATURATION_MARGIN); + } else if (!hsbColorA.hasSaturation() && !hsbColorB.hasSaturation()) { + saturationEquals = true; + } else { + // one missing: consider equal if the present saturation is effectively 0 (neutral) + // or if the present has no saturation but brightness is present and 0 -> neutral + final HSBColor present = hsbColorA.hasSaturation() ? hsbColorA : hsbColorB; + boolean presentIsNeutral = false; + if (present.hasSaturation()) { + presentIsNeutral = OperationService.equals(present.getSaturation(), 0d, SATURATION_MARGIN); + } else if (present.hasBrightness()) { + presentIsNeutral = OperationService.equals(present.getBrightness(), 0d, BRIGHTNESS_MARGIN); + } else { + // no saturation and no brightness information -> treat as neutral + presentIsNeutral = true; + } + saturationEquals = presentIsNeutral; } - if(hsbColorA.hasBrightness() && hsbColorB.hasBrightness()) { - brightnessEquals = OperationService.equals(hsbColorA.getBrightness(), hsbColorB.getBrightness(), 0.01); + boolean brightnessEquals; + if (hsbColorA.hasBrightness() && hsbColorB.hasBrightness()) { + brightnessEquals = OperationService.equals(hsbColorA.getBrightness(), hsbColorB.getBrightness(), BRIGHTNESS_MARGIN); + } else if (!hsbColorA.hasBrightness() && !hsbColorB.hasBrightness()) { + brightnessEquals = true; + } else { + // one missing: consider equal if the present brightness is effectively 0 (off/neutral) + // or if the present has no brightness but saturation is present and 0 -> neutral + final HSBColor present = hsbColorA.hasBrightness() ? hsbColorA : hsbColorB; + boolean presentIsNeutral = false; + if (present.hasBrightness()) { + presentIsNeutral = OperationService.equals(present.getBrightness(), 0d, BRIGHTNESS_MARGIN); + } else if (present.hasSaturation()) { + presentIsNeutral = OperationService.equals(present.getSaturation(), 0d, SATURATION_MARGIN); + } else { + // no brightness and no saturation information -> treat as neutral + presentIsNeutral = true; + } + brightnessEquals = presentIsNeutral; } return hueEquals && saturationEquals && brightnessEquals; diff --git a/module/dal/lib/src/test/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderServiceTest.java b/module/dal/lib/src/test/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderServiceTest.java deleted file mode 100644 index c7a9b9d05d..0000000000 --- a/module/dal/lib/src/test/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderServiceTest.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.openbase.bco.dal.lib.layer.service.provider; - -/*- - * #%L - * BCO DAL Library - * %% - * Copyright (C) 2014 - 2021 openbase.org - * %% - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Lesser Public License for more details. - * - * You should have received a copy of the GNU General Lesser Public - * License along with this program. If not, see - * . - * #L% - */ - -import static org.junit.jupiter.api.Assertions.*; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Timeout; -import org.openbase.jps.core.JPService; -import org.openbase.jps.exception.JPServiceException; -import org.openbase.jul.exception.VerificationFailedException; -import org.openbase.jul.exception.printer.ExceptionPrinter; -import org.openbase.type.domotic.state.ColorStateType.ColorState; -import org.openbase.type.domotic.state.ColorStateType.ColorState.Builder; - -public class ColorStateProviderServiceTest { - - @Test - @Timeout(10) - public void verifyColorState() throws VerificationFailedException, JPServiceException { - - JPService.setupJUnitTestMode(); - - final Builder builder = ColorState.newBuilder(); - builder.getColorBuilder().getHsbColorBuilder().setHue(240); - builder.getColorBuilder().getHsbColorBuilder().setSaturation(100); - builder.getColorBuilder().getHsbColorBuilder().setBrightness(50); - - ExceptionPrinter.setBeQuit(true); - final ColorState verifiedColorState = ColorStateProviderService.verifyColorState(builder.build()); - ExceptionPrinter.setBeQuit(false); - - assertEquals(builder.getColorBuilder().getHsbColorBuilder().getHue(), verifiedColorState.getColor().getHsbColor().getHue(), 0.00001, "Hue value invalid!"); - assertEquals(1d, verifiedColorState.getColor().getHsbColor().getSaturation(), 0.00001, "Hue value invalid!"); - assertEquals(0.5d, verifiedColorState.getColor().getHsbColor().getBrightness(), 0.00001, "Hue value invalid!"); - } -} diff --git a/module/dal/lib/src/test/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderServiceTest.kt b/module/dal/lib/src/test/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderServiceTest.kt new file mode 100644 index 0000000000..8dd28e294b --- /dev/null +++ b/module/dal/lib/src/test/java/org/openbase/bco/dal/lib/layer/service/provider/ColorStateProviderServiceTest.kt @@ -0,0 +1,342 @@ +package org.openbase.bco.dal.lib.layer.service.provider + +import io.kotest.matchers.shouldBe +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.Timeout +import org.openbase.jps.core.JPService +import org.openbase.jps.exception.JPServiceException +import org.openbase.jul.exception.VerificationFailedException +import org.openbase.jul.exception.printer.ExceptionPrinter +import org.openbase.type.domotic.state.ColorStateType.ColorState + +/*- +* #%L +* BCO DAL Library +* %% +* Copyright (C) 2014 - 2021 openbase.org +* %% +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU Lesser General Public License as +* published by the Free Software Foundation, either version 3 of the +* License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Lesser Public License for more details. +* +* You should have received a copy of the GNU General Lesser Public +* License along with this program. If not, see +* . +* #L% +*/ + +class ColorStateProviderServiceTest { + @Test + @Timeout(10) + fun `should fix color values once they are defined out of range`() { + JPService.setupJUnitTestMode() + + val builder = ColorState.newBuilder() + builder.getColorBuilder().getHsbColorBuilder().setHue(240.0) + builder.getColorBuilder().getHsbColorBuilder().setSaturation(100.0) + builder.getColorBuilder().getHsbColorBuilder().setBrightness(50.0) + + ExceptionPrinter.setBeQuit(true) + val verifiedColorState = ColorStateProviderService.verifyColorState(builder.build()) + ExceptionPrinter.setBeQuit(false) + + Assertions.assertEquals( + builder.getColorBuilder().getHsbColorBuilder().getHue(), + verifiedColorState.getColor().getHsbColor().getHue(), + 0.00001, + "Hue value invalid!" + ) + Assertions.assertEquals( + 1.0, + verifiedColorState.getColor().getHsbColor().getSaturation(), + 0.00001, + "Hue value invalid!" + ) + Assertions.assertEquals( + 0.5, + verifiedColorState.getColor().getHsbColor().getBrightness(), + 0.00001, + "Hue value invalid!" + ) + } + + @Test + @Timeout(10) + fun `should handle color state comparison with equal state`() { + JPService.setupJUnitTestMode() + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe true + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(0.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(360.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe true + } + + @Test + @Timeout(10) + fun `should handle color state comparison with non equal state`() { + JPService.setupJUnitTestMode() + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(50.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(100.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(30.0) + setSaturation(0.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(50.0) + setBrightness(1.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(10.0) + setSaturation(100.0) + setBrightness(20.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(233.0) + setSaturation(23.0) + setBrightness(12.0) + } + }.build(), + ) shouldBe false + + } + + @Test + @Timeout(10) + fun `should handle color state comparison with neutral state`() { + JPService.setupJUnitTestMode() + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setBrightness(50.0) + } + }.build(), + ) shouldBe false + + ColorStateProviderService.equalServiceStates( + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + setBrightness(50.0) + } + }.build(), + ColorState.newBuilder().apply { + getColorBuilder().getHsbColorBuilder().apply { + setHue(240.0) + setSaturation(100.0) + } + }.build(), + ) shouldBe false + } +} diff --git a/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/action/RemoteAction.java b/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/action/RemoteAction.java index 0ea35e8516..5bf89348da 100644 --- a/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/action/RemoteAction.java +++ b/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/action/RemoteAction.java @@ -619,7 +619,7 @@ public boolean isRunning() { try { if (getActionDescription().getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { if (impactedRemoteAction.isRunning()) { return true; } @@ -650,7 +650,7 @@ public boolean isDone() { try { if (getActionDescription().getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { if (!impactedRemoteAction.isDone()) { return false; } @@ -854,7 +854,7 @@ private void cleanup() { // cleanup synchronisation and observation tasks actionDescriptionObservable.reset(); - for (RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { impactedRemoteAction.removeActionDescriptionObserver(impactActionObserver); } impactedRemoteActions.clear(); @@ -955,7 +955,7 @@ public void waitUntilDone() throws CouldNotPerformException, InterruptedExceptio try { if (getActionDescription().getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { impactedRemoteAction.waitUntilDone(); } return; @@ -1006,7 +1006,7 @@ public void waitForActionState(final ActionState.State actionState, final long t try { if (actionDescription.getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { impactedRemoteAction.waitForActionState(actionState, timeSplit.getTime(), timeSplit.getTimeUnit()); } return; @@ -1027,7 +1027,9 @@ public void waitForActionState(final ActionState.State actionState, final long t while (actionDescription == null || (actionDescription.getActionState().getValue() != actionState) && !checkIfStateWasPassed(actionState, timeSplit.getTimestamp(), actionDescription)) { // Waiting makes no sense if the action is done but the state is still not reached. if (actionDescription != null && isDone()) { - throw new CouldNotPerformException(targetUnit.getLabel() + " - stop waiting because state[" + actionState.name() + "] cannot be reached from state[" + actionDescription.getActionState().getValue().name() + "]"); + final State currentState = actionDescription.getActionState().getValue(); + LOGGER.warn(getTargetUnit().getLabel() + " - Action [" + this + "] reached terminal state [" + currentState.name() + "] instead of target state [" + actionState.name() + "]"); + throw new CouldNotPerformException(targetUnit.getLabel() + " - stop waiting because state[" + actionState.name() + "] cannot be reached from state[" + currentState.name() + "]"); } LOGGER.trace(getTargetUnit().getLabel() + " - wait for action [" + this + "] to be in state [" + actionState + "] "); executionSync.wait(timeSplit.getTime()); @@ -1091,7 +1093,7 @@ public void waitForRegistration() throws CouldNotPerformException, InterruptedEx } if (getActionDescription().getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { impactedRemoteAction.waitForRegistration(); } } @@ -1130,7 +1132,7 @@ public void waitForRegistration(final long timeout, final TimeUnit timeUnit) thr } if (getActionDescription().getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { impactedRemoteAction.waitForRegistration(timeSplit.getTime(), timeSplit.getTimeUnit()); } } @@ -1147,7 +1149,7 @@ public boolean isRegistrationDone() { } if (actionDescription.getIntermediary()) { - for (final RemoteAction impactedRemoteAction : impactedRemoteActions) { + for (final RemoteAction impactedRemoteAction : new ArrayList<>(impactedRemoteActions)) { if (!impactedRemoteAction.isRegistrationDone()) { return false; } diff --git a/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/service/AbstractServiceRemote.java b/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/service/AbstractServiceRemote.java index e43815ed42..c617344ebb 100644 --- a/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/service/AbstractServiceRemote.java +++ b/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/service/AbstractServiceRemote.java @@ -75,6 +75,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import static org.openbase.bco.dal.lib.layer.service.ServiceStateProcessor.*; @@ -107,7 +108,7 @@ public abstract class AbstractServiceRemote, ST> serviceStateObservable = new ObservableImpl<>(); private final ObservableImpl, ST> serviceStateProviderObservable = new ObservableImpl<>(); - private final SyncObject syncObject = new SyncObject("ServiceStateComputationLock"); + private final ReentrantLock lock = new ReentrantLock(); private final SyncObject maintainerLock = new SyncObject("MaintainerLock"); private final SyncObject connectionStateLock = new SyncObject("ConnectionStateLock"); protected Object maintainer; @@ -181,8 +182,11 @@ public AbstractServiceRemote(final ServiceType serviceType, final Class serv */ private void updateServiceState() throws CouldNotPerformException { final ST serviceState; - synchronized (syncObject) { + lock.lock(); + try { serviceState = computeServiceState(); + } finally { + lock.unlock(); } serviceStateObservable.notifyObservers(serviceState); serviceStateProviderObservable.notifyObservers(serviceState); @@ -197,12 +201,10 @@ private void updateServiceState() throws CouldNotPerformException { */ @Override public ST getData() throws NotAvailableException { -// synchronized (syncObject) { if (!serviceStateObservable.isValueAvailable()) { throw new NotAvailableException("Data"); } return serviceStateObservable.getValue(); -// } } @Override diff --git a/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/AbstractUnitRemote.java b/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/AbstractUnitRemote.java index e0801800b8..90a328cc26 100644 --- a/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/AbstractUnitRemote.java +++ b/module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/AbstractUnitRemote.java @@ -99,6 +99,8 @@ public abstract class AbstractUnitRemote extends AbstractAuth private boolean initialized = false; private SessionManager sessionManager; private boolean infrastructure = false; + private volatile String cachedId = null; + private volatile String cachedLabel = null; public AbstractUnitRemote(final Class dataClass) { super(dataClass, UnitConfig.class); @@ -329,6 +331,10 @@ public UnitConfig applyConfigUpdate(final UnitConfig unitConfig) throws CouldNot logger.trace("Unit config change check failed because config is not available yet."); } + // clear caches + cachedId = null; + cachedLabel = null; + // update unit templates unitTemplate = Registries.getTemplateRegistry(true).getUnitTemplateByType(Units.getUnitTypeByRemoteClass((Class>) getClass())); @@ -494,17 +500,22 @@ public UnitTemplate getUnitTemplate() throws NotAvailableException { */ @Override public String getLabel() throws NotAvailableException { + if (cachedLabel != null) { + return cachedLabel; + } try { if (getSessionManager().isLoggedIn()) { try { UnitConfig user = Registries.getUnitRegistry().getUnitConfigById(getSessionManager().getUserClientPair().getUserId()); - return LabelProcessor.getLabelByLanguage(user.getUserConfig().getLanguage(), getConfig().getLabel()); + this.cachedLabel = LabelProcessor.getLabelByLanguage(user.getUserConfig().getLanguage(), getConfig().getLabel()); + return cachedLabel; } catch (CouldNotPerformException ex) { // as a backup use the best match result //TODO: this should parse a value from the root location meta config that defines a default label lang for this smart environment. } } - return LabelProcessor.getBestMatch(getConfig().getLabel()); + this.cachedLabel = LabelProcessor.getBestMatch(getConfig().getLabel()); + return cachedLabel; } catch (NullPointerException | NotAvailableException ex) { throw new NotAvailableException("unit label", ex); } diff --git a/module/dal/test/src/main/java/org/openbase/bco/dal/test/AbstractBCOTest.java b/module/dal/test/src/main/java/org/openbase/bco/dal/test/AbstractBCOTest.java index 80f9ab06b8..c379d499d2 100644 --- a/module/dal/test/src/main/java/org/openbase/bco/dal/test/AbstractBCOTest.java +++ b/module/dal/test/src/main/java/org/openbase/bco/dal/test/AbstractBCOTest.java @@ -32,6 +32,7 @@ import org.openbase.bco.registry.mock.MockRegistryHolder; import org.openbase.jul.communication.mqtt.test.MqttIntegrationTest; import org.openbase.jul.exception.CouldNotPerformException; +import org.openbase.jul.exception.NotAvailableException; import org.openbase.jul.exception.StackTracePrinter; import org.openbase.jul.exception.printer.ExceptionPrinter; import org.openbase.type.domotic.action.ActionDescriptionType.ActionDescription; @@ -86,6 +87,8 @@ public static void tearDownBCO() throws Throwable { @Timeout(30) public void notifyAboutTestStart() { LOGGER.info("===================================== Start BCO Test ====================================="); + LOGGER.debug("Test class: " + getClass().getSimpleName()); + LOGGER.debug("Active test actions before test execution: " + testActions.size()); } /** @@ -97,6 +100,7 @@ public void notifyAboutTestStart() { public void autoCancelActionsAfterTestRun() { LOGGER.info("===================================== Finish BCO Test ====================================="); + LOGGER.debug("Active test actions after test execution: " + testActions.size()); // before canceling pending actions lets just validate that the test did not cause any deadlocks assertFalse(StackTracePrinter.detectDeadLocksAndPrintStackTraces(LOGGER), "Deadlocks found!"); @@ -374,9 +378,14 @@ public RemoteAction observe(final RemoteAction remoteAction) { // register current action. testActions.add(remoteAction); + LOGGER.debug("Registered test action. Total active actions: " + testActions.size()); // cleanup finished actions + final int beforeCleanup = testActions.size(); testActions.removeIf(RemoteAction::isDone); + if (beforeCleanup > testActions.size()) { + LOGGER.debug("Cleaned up " + (beforeCleanup - testActions.size()) + " finished test action(s). Remaining: " + testActions.size()); + } return remoteAction; } @@ -392,6 +401,30 @@ public void cancelAllTestActions() throws CouldNotPerformException, InterruptedE if (testActions.size() > 0) { LOGGER.info("Cancel " + testActions.size() + " ongoing test action" + (testActions.size() == 1 ? "" : "s") + " ..."); + + // Log action states for debugging + int canceledCount = 0; + int rejectedCount = 0; + int finishedCount = 0; + for (RemoteAction action : testActions) { + final State state = action.getActionState(); + switch (state) { + case CANCELED: + canceledCount++; + break; + case REJECTED: + rejectedCount++; + break; + case FINISHED: + finishedCount++; + break; + default: + LOGGER.debug("Action in state: " + state); + } + } + if (canceledCount > 0 || rejectedCount > 0 || finishedCount > 0) { + LOGGER.debug("Action state summary - Canceled: " + canceledCount + ", Rejected: " + rejectedCount + ", Finished: " + finishedCount); + } } final List> cancelTasks = new ArrayList<>(); diff --git a/module/dal/test/src/test/java/org/openbase/bco/dal/test/action/RemoteActionTest.kt b/module/dal/test/src/test/java/org/openbase/bco/dal/test/action/RemoteActionTest.kt index c5383f0d5b..deb4196592 100644 --- a/module/dal/test/src/test/java/org/openbase/bco/dal/test/action/RemoteActionTest.kt +++ b/module/dal/test/src/test/java/org/openbase/bco/dal/test/action/RemoteActionTest.kt @@ -164,7 +164,7 @@ class RemoteActionTest : AbstractBCOLocationManagerTest() { } @Test - @Timeout(15) + @Timeout(60) @Throws(Exception::class) fun testExtensionCancellation() { println("testExtensionCancellation") diff --git a/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/HighFrequentModificationTest.kt b/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/HighFrequentModificationTest.kt new file mode 100644 index 0000000000..ab901c7e2e --- /dev/null +++ b/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/HighFrequentModificationTest.kt @@ -0,0 +1,131 @@ +package org.openbase.bco.dal.test.layer.unit + +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.RepeatedTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.Timeout +import org.openbase.bco.dal.lib.state.States +import org.openbase.bco.dal.remote.layer.unit.LightRemote +import org.openbase.bco.dal.remote.layer.unit.Units +import org.openbase.bco.dal.test.AbstractBCODeviceManagerTest +import org.openbase.bco.registry.mock.MockRegistry +import org.openbase.bco.registry.remote.Registries +import org.openbase.jul.extension.type.processing.LabelProcessor +import org.openbase.type.domotic.unit.UnitTemplateType.UnitTemplate +import java.util.logging.Level +import java.util.logging.Logger + +/** + * @author [Tamino Huxohl](mailto:pleminoq@openbase.org) + */ +class HighFrequentModificationTest : AbstractBCODeviceManagerTest() { + + var lightRemote: LightRemote? = null + + @BeforeAll + @Timeout(30) + @Throws(Throwable::class) + fun loadUnits() { + lightRemote = Units.getUnitByAlias( + MockRegistry.getUnitAlias(UnitTemplate.UnitType.LIGHT), + true, + LightRemote::class.java + ) + } + + /** + * Test to simulate race conditions by modifying the Light with 4 threads in parallel. + * + * This test creates four threads that perform concurrent modifications on the LightRemote instance: + * - Two threads toggle the PowerState randomly between ON and OFF. + * - Two threads modify the UnitConfig (label and location) of the light. + * + * This test aims to expose potential race conditions in the LightRemote implementation. + * + * @throws Exception if any thread encounters an error during execution. + */ + @RepeatedTest(10) + @Timeout(30) + @Throws(Exception::class) + fun `high frequent config and service modification should should be possible`() { + println("testRaceCondition") + + val numberOfInterations = 10 + + // Thread 1: Randomly toggle PowerState ON/OFF + val powerStateThread1 = Thread(Runnable { + try { + for (i in 0..numberOfInterations) { + lightRemote!!.setPowerState(if (Math.random() > 0.5) States.Power.ON else States.Power.OFF) + } + } catch (e: Exception) { + LOGGER.log(Level.SEVERE, "Error in powerStateThread1", e) + } + }) + + // Thread 2: Randomly toggle PowerState ON/OFF + val powerStateThread2 = Thread(Runnable { + try { + for (i in 0..numberOfInterations) { + lightRemote!!.setPowerState(if (Math.random() > 0.5) States.Power.ON else States.Power.OFF).get() + } + } catch (e: Exception) { + LOGGER.log(Level.SEVERE, "Error in powerStateThread2", e) + } + }) + + // Thread 3: Modify the label of the unit + val unitConfigThread1 = Thread(Runnable { + try { + for (i in 0..numberOfInterations) { + val newLabel = LabelProcessor.buildLabel("Label$i") + Registries.getUnitRegistry().getUnitConfigById(lightRemote!!.getId()) + .toBuilder() + .apply { setLabel(newLabel) } + .build() + .also { Registries.getUnitRegistry().updateUnitConfig(it) } + } + } catch (e: Exception) { + LOGGER.log(Level.SEVERE, "Error in unitConfigThread1", e) + } + }) + + // Thread 4: Modify the location of the unit + val unitConfigThread2 = Thread(Runnable { + try { + for (i in 0..numberOfInterations) { + val newLocationId = Registries.getUnitRegistry().getUnitConfigsByUnitType(UnitTemplate.UnitType.LOCATION) + .shuffled() + .first() + .id + + Registries.getUnitRegistry().getUnitConfigById(lightRemote!!.getId()) + .toBuilder() + .apply { placementConfigBuilder.setLocationId(newLocationId) } + .build() + .also { Registries.getUnitRegistry().updateUnitConfig(it) } + } + } catch (e: Exception) { + LOGGER.log(Level.SEVERE, "Error in unitConfigThread2", e) + } + }) + + // Start all threads + powerStateThread1.start() + powerStateThread2.start() + unitConfigThread1.start() + unitConfigThread2.start() + + // Wait for all threads to complete + powerStateThread1.join() + powerStateThread2.join() + unitConfigThread1.join() + unitConfigThread2.join() + + println("Race condition test completed.") + } + + companion object { + private val LOGGER: Logger = Logger.getLogger(HighFrequentModificationTest::class.java.getName()) + } +} diff --git a/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/scene/SceneRemoteTest.java b/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/scene/SceneRemoteTest.java index 1de16c1664..838c2acb46 100644 --- a/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/scene/SceneRemoteTest.java +++ b/module/dal/test/src/test/java/org/openbase/bco/dal/test/layer/unit/scene/SceneRemoteTest.java @@ -27,6 +27,7 @@ import org.openbase.bco.dal.control.layer.unit.device.DeviceManagerLauncher; import org.openbase.bco.dal.control.layer.unit.location.LocationManagerLauncher; import org.openbase.bco.dal.control.layer.unit.scene.SceneManagerLauncher; +import org.openbase.bco.dal.lib.action.Action; import org.openbase.bco.dal.lib.layer.service.ServiceJSonProcessor; import org.openbase.bco.dal.lib.layer.service.Services; import org.openbase.bco.dal.lib.layer.service.provider.ColorStateProviderService; @@ -56,6 +57,7 @@ import org.openbase.jul.exception.printer.ExceptionPrinter; import org.openbase.jul.extension.type.processing.LabelProcessor; import org.openbase.jul.extension.type.processing.MultiLanguageTextProcessor; +import org.slf4j.LoggerFactory; import org.openbase.type.domotic.action.ActionDescriptionType; import org.openbase.type.domotic.action.ActionDescriptionType.ActionDescription; import org.openbase.type.domotic.action.ActionParameterType.ActionParameter; @@ -453,7 +455,22 @@ public void testTriggerUnitGroupByScene() throws Exception { for (String memberId : unitGroupRemote.getConfig().getUnitGroupConfig().getMemberIdList()) { colorableLightRemotes.add(Units.getUnit(memberId, true, ColorableLightRemote.class)); } - waitForExecution(sceneRemote.setActivationState(State.ACTIVE)); + + var actionFuture = sceneRemote.setActivationState(State.ACTIVE); + + // Try to activate scene with error handling for canceled actions + try { + waitForExecution(actionFuture); + } catch (CouldNotPerformException ex) { + var action = actionFuture.get(); + + // Log action state if available to help with diagnostics + final ActionDescription currentAction = sceneRemote.getActionList().getFirst(); + LoggerFactory.getLogger(SceneRemoteTest.class).error( + "Scene activation was " + action.getActionState().getValue().name() + " (likely due to higher priority actions): " + + currentAction.getDescription(), ex); + throw ex; + } for (ColorableLightRemote colorableLightRemote : colorableLightRemotes) { assertEquals(GROUP_COLOR_VALUE, colorableLightRemote.getColorState().getColor().getHsbColor(), "ColorState has not been set for light[" + colorableLightRemote.getLabel() + "]"); diff --git a/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/UnitRegistryController.java b/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/UnitRegistryController.java index af83257003..0777b26595 100644 --- a/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/UnitRegistryController.java +++ b/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/UnitRegistryController.java @@ -115,15 +115,12 @@ public class UnitRegistryController extends AbstractRegistryController> unitConfigRegistryList, baseUnitConfigRegistryList; - private final SyncObject aliasIdMapLock; - private final TreeMap aliasIdMap; + private final SyncObject aliasIdMapLock = new SyncObject("AliasIdMapLock"); + private final TreeMap aliasIdMap = new TreeMap<>(); public UnitRegistryController() throws InstantiationException, InterruptedException { super(JPUnitRegistryScope.class, UnitRegistryData.newBuilder()); try { - this.aliasIdMap = new TreeMap<>(); - this.aliasIdMapLock = new SyncObject("AliasIdMapLock"); - this.unitConfigRegistryList = new ArrayList<>(); this.baseUnitConfigRegistryList = new ArrayList<>(); @@ -185,14 +182,19 @@ protected void postInit() throws InitializationException, InterruptedException { // initially fill the alias to id map afterwards // the {@code AliasMapUpdatePlugin} will manage changes on registering, removing or updating of units - synchronized (aliasIdMapLock) { - try { - for (ProtoBufFileSynchronizedRegistry registry : unitConfigRegistryList) { - registry.getMessages().forEach((unitConfig) -> unitConfig.getAliasList().forEach(alias -> aliasIdMap.put(alias.toLowerCase(), unitConfig.getId()))); - } - } catch (CouldNotPerformException ex) { - throw new InitializationException(this, ex); + + TreeMap tempAliasIdMap = new TreeMap<>(); + + try { + for (ProtoBufFileSynchronizedRegistry registry : unitConfigRegistryList) { + registry.getMessages().forEach((unitConfig) -> unitConfig.getAliasList().forEach(alias -> tempAliasIdMap.put(alias.toLowerCase(), unitConfig.getId()))); } + } catch (CouldNotPerformException ex) { + throw new InitializationException(this, ex); + } + + synchronized (aliasIdMapLock){ + aliasIdMap.putAll(tempAliasIdMap); } } @@ -585,12 +587,15 @@ public UnitConfig getUnitConfigById(final String unitConfigId) throws NotAvailab @Override public UnitConfig getUnitConfigByAlias(final String unitAlias) throws NotAvailableException { try { + String unitId; synchronized (aliasIdMapLock) { if (aliasIdMap.containsKey(unitAlias.toLowerCase())) { - return getUnitConfigById(aliasIdMap.get(unitAlias.toLowerCase())); + unitId = aliasIdMap.get(unitAlias.toLowerCase()); + } else { + throw new NotAvailableException("Alias", unitAlias); } } - throw new NotAvailableException("Alias", unitAlias); + return getUnitConfigById(unitId); } catch (CouldNotPerformException ex) { throw new NotAvailableException("UnitConfig with Alias", unitAlias, ex); } @@ -609,12 +614,15 @@ public UnitConfig getUnitConfigByAlias(final String unitAlias) throws NotAvailab @Override public UnitConfig getUnitConfigByAliasAndUnitType(final String unitAlias, final UnitType unitType) throws NotAvailableException { try { + String unitId; synchronized (aliasIdMapLock) { if (aliasIdMap.containsKey(unitAlias.toLowerCase())) { - return getUnitConfigByIdAndUnitType(aliasIdMap.get(unitAlias.toLowerCase()), unitType); + unitId = aliasIdMap.get(unitAlias.toLowerCase()); + } else { + throw new NotAvailableException("Alias", unitAlias); } } - throw new NotAvailableException("Alias", unitAlias); + return getUnitConfigByIdAndUnitType(unitId, unitType); } catch (CouldNotPerformException ex) { throw new NotAvailableException("UnitConfig of UnitType[" + unitType.name() + "] with Alias", unitAlias, ex); } diff --git a/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/plugin/AliasMapUpdatePlugin.java b/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/plugin/AliasMapUpdatePlugin.java index 709ab4cd9a..f19d5da47b 100644 --- a/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/plugin/AliasMapUpdatePlugin.java +++ b/module/registry/unit-registry/core/src/main/java/org/openbase/bco/registry/unit/core/plugin/AliasMapUpdatePlugin.java @@ -95,9 +95,7 @@ public void afterUpdate(IdentifiableMessage identif aliasIdMap.remove(alias.toLowerCase()); } }); + temporaryAliasList.clear(); } - - temporaryAliasList.clear(); } - } diff --git a/module/registry/unit-registry/remote/src/main/java/org/openbase/bco/registry/unit/remote/UnitRegistryRemote.java b/module/registry/unit-registry/remote/src/main/java/org/openbase/bco/registry/unit/remote/UnitRegistryRemote.java index 5ce54eb604..3dcf6029a4 100644 --- a/module/registry/unit-registry/remote/src/main/java/org/openbase/bco/registry/unit/remote/UnitRegistryRemote.java +++ b/module/registry/unit-registry/remote/src/main/java/org/openbase/bco/registry/unit/remote/UnitRegistryRemote.java @@ -129,14 +129,16 @@ public UnitRegistryRemote() throws InstantiationException { ); aliasMapUpdateObserver = (source, data) -> { + TreeMap tempAliasIdMap = new TreeMap<>(); + for (IdentifiableMessage identifiableMessage : data.values()) { + final UnitConfig unitConfig = identifiableMessage.getMessage(); + for (String alias : unitConfig.getAliasList()) { + tempAliasIdMap.put(alias.toLowerCase(), unitConfig.getId()); + } + } synchronized (aliasIdMapLock) { aliasIdMap.clear(); - for (IdentifiableMessage identifiableMessage : data.values()) { - final UnitConfig unitConfig = identifiableMessage.getMessage(); - for (String alias : unitConfig.getAliasList()) { - aliasIdMap.put(alias.toLowerCase(), unitConfig.getId()); - } - } + aliasIdMap.putAll(tempAliasIdMap); } }; } catch (CouldNotPerformException ex) { @@ -388,12 +390,15 @@ public UnitConfig getUnitConfigById(final String unitConfigId) throws NotAvailab @Override public UnitConfig getUnitConfigByAlias(final String unitAlias) throws NotAvailableException { try { + String unitId; synchronized (aliasIdMapLock) { if (aliasIdMap.containsKey(unitAlias.toLowerCase())) { - return getUnitConfigById(aliasIdMap.get(unitAlias.toLowerCase())); + unitId = aliasIdMap.get(unitAlias.toLowerCase()); + } else { + throw new NotAvailableException("Alias", unitAlias); } } - throw new NotAvailableException("Alias", unitAlias); + return getUnitConfigById(unitId); } catch (CouldNotPerformException ex) { throw new NotAvailableException("UnitConfig with Alias", unitAlias, ex); } @@ -412,12 +417,15 @@ public UnitConfig getUnitConfigByAlias(final String unitAlias) throws NotAvailab @Override public UnitConfig getUnitConfigByAliasAndUnitType(String unitAlias, final UnitType unitType) throws NotAvailableException { try { + String unitId; synchronized (aliasIdMapLock) { if (aliasIdMap.containsKey(unitAlias.toLowerCase())) { - return getUnitConfigByIdAndUnitType(aliasIdMap.get(unitAlias.toLowerCase()), unitType); + unitId = aliasIdMap.get(unitAlias.toLowerCase()); + } else { + throw new NotAvailableException("Alias", unitAlias); } } - throw new NotAvailableException("Alias", unitAlias); + return getUnitConfigByIdAndUnitType(unitId, unitType); } catch (CouldNotPerformException ex) { throw new NotAvailableException("UnitConfig of UnitType[" + unitType.name() + "] with Alias", unitAlias, ex); } diff --git a/module/registry/unit-registry/test/src/test/java/org/openbase/bco/registry/unit/test/TestBoundToDeviceFlag.java b/module/registry/unit-registry/test/src/test/java/org/openbase/bco/registry/unit/test/TestBoundToDeviceFlag.java index fde29a9dca..b43fb804b3 100644 --- a/module/registry/unit-registry/test/src/test/java/org/openbase/bco/registry/unit/test/TestBoundToDeviceFlag.java +++ b/module/registry/unit-registry/test/src/test/java/org/openbase/bco/registry/unit/test/TestBoundToDeviceFlag.java @@ -59,7 +59,7 @@ public class TestBoundToDeviceFlag extends AbstractBCORegistryTest { private Pose poseLightThree; @BeforeEach - @Timeout(30) + @Timeout(60) public void setupTest() throws Exception { try { deviceClass = Registries.getClassRegistry().registerDeviceClass(generateDeviceClass("Label", "Product Number", "Company", unitTypes)).get(); diff --git a/settings.gradle.kts b/settings.gradle.kts index 1f08013f17..60654c2f2b 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -10,11 +10,6 @@ plugins { id("de.fayard.refreshVersions") } -include("authentication") -include("registry") -include("dal") -include("device") - include(":bco.authentication.test") include(":bco.authentication.lib") include(":bco.authentication.core") diff --git a/versions.properties b/versions.properties index 3465159491..8a1d411984 100644 --- a/versions.properties +++ b/versions.properties @@ -72,9 +72,9 @@ version.com.google.guava..guava=28.0-jre ## # available=31.0.1-android ## # available=31.0.1-jre -version.org.openbase..jul.communication.mqtt.test=3.7.2 +version.org.openbase..jul.communication.mqtt.test=3.7.4 -version.org.openbase..jul.transformation=3.7.2 +version.org.openbase..jul.transformation=3.7.4 version.org.testcontainers..junit-jupiter=1.21.4 @@ -98,74 +98,32 @@ version.org.springframework.boot..spring-boot-starter-actuator=3.1.2 version.kotlin=2.0.0 -version.org.openbase..jul.communication.controller=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.communication.mqtt=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.exception=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.extension.protobuf=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.extension.type.processing=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.extension.type.storage=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.extension.type.transform=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.extension.type.util=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.pattern.launch=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.pattern.trigger=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.processing=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.storage=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.visual.javafx=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 - -version.org.openbase..jul.visual.swing=3.7.2 -## # available=3.0.0 -## # available=3.0.1 -## # available=3.0.2 +version.org.openbase..jul.communication.controller=3.7.4 + +version.org.openbase..jul.communication.mqtt=3.7.4 + +version.org.openbase..jul.exception=3.7.4 + +version.org.openbase..jul.extension.protobuf=3.7.4 + +version.org.openbase..jul.extension.type.processing=3.7.4 + +version.org.openbase..jul.extension.type.storage=3.7.4 + +version.org.openbase..jul.extension.type.transform=3.7.4 + +version.org.openbase..jul.extension.type.util=3.7.4 + +version.org.openbase..jul.pattern.launch=3.7.4 + +version.org.openbase..jul.pattern.trigger=3.7.4 + +version.org.openbase..jul.processing=3.7.4 + +version.org.openbase..jul.storage=3.7.4 + +version.org.openbase..jul.visual.javafx=3.7.4 + +version.org.openbase..jul.visual.swing=3.7.4 version.rxjava2.rxjava=2.2.21