diff --git a/.github/workflows/compile-check.yml b/.github/workflows/compile-check.yml index afb21d9..801631c 100644 --- a/.github/workflows/compile-check.yml +++ b/.github/workflows/compile-check.yml @@ -37,6 +37,7 @@ jobs: java: [8, 11, 17, 21] os: [ ubuntu-latest ] runs-on: ${{ matrix.os }} + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - name: Set up JDK ${{ matrix.java }} diff --git a/iotdb-thingsboard-table/README.md b/iotdb-thingsboard-table/README.md index 00f0c76..9e0b811 100644 --- a/iotdb-thingsboard-table/README.md +++ b/iotdb-thingsboard-table/README.md @@ -74,7 +74,8 @@ selector: `telemetry_latest` overlay. Enabled by `database.ts_latest.type=iotdb-table` (see the latest-telemetry section below). - `IoTDBTableAttributesDao`: entity attributes, **inert by default**. Enabled - only by the independent `database.attributes.type=iotdb-table` opt-in. + only by the `database.attributes.type=iotdb-table` opt-in, which is separate + from the two timeseries selectors. > **This is an incremental / experimental backend.** Nothing routes through > IoTDB Table Mode unless the matching selector is set explicitly; with no @@ -91,11 +92,19 @@ explicitly with `database.attributes.type=iotdb-table`. This attribute selector is **independent** of `database.ts.type` / `database.ts_latest.type` — the attribute DAO routes separately from the time-series DAOs (a piggy-back on the timeseries selector was deliberately -rejected). No shipped ThingsBoard release exposes a `database.attributes.type` -selector yet, so a real Phase-1 deployment never sets it; the activation condition +rejected). Leaving it unset is the default posture: the activation condition stays false, no attribute bean or session pool is created, and attributes keep flowing to the host entity-DB `AttributesDao`. +`database.attributes.type` is a selector this module supplies rather than one +ThingsBoard offers. ThingsBoard switches its timeseries DAOs by configuration but +has no equivalent for attributes — at v4.3.1.2 its `JpaAttributeDao` is an +unconditional `@Component`, so no property can stand it down. Setting this +selector therefore has the module withdraw that one bean at startup, matched on +both its bean name and its fully-qualified class name, logging a WARN that names +it; any other competing `AttributesDao` fails startup untouched. See +`docs/user-guide.md` for the full semantics and their boundary. + When activated, each identity tuple `(tenant_id, entity_type, entity_id, attribute_scope, key)` holds exactly one current row: `save` is a tag-only `DELETE` (no time predicate) followed by an @@ -302,7 +311,7 @@ Key activation and operational flags: | --- | --- | --- | | `database.ts.type` | _(unset)_ | Set to `iotdb-table` as the ThingsBoard historical-timeseries backend selector. | | `iotdb.ts.experimental-raw-only` | `false` | Explicit opt-in for this backend. Must be `true` together with `database.ts.type=iotdb-table`. The name predates the aggregation support and is kept for compatibility: write, raw read, delete **and** time-bucketed aggregation are all served when it is enabled. | -| `database.attributes.type` | _(unset)_ | Set to `iotdb-table` to opt in to the entity-attribute DAO. Independent of the timeseries selectors. Unset in a real Phase-1 deployment, so the attribute DAO is inert by default. | +| `database.attributes.type` | _(unset)_ | Set to `iotdb-table` to opt in to the entity-attribute DAO. Independent of the timeseries selectors. Unset by default, and while unset the attribute DAO is inert. Setting it withdraws ThingsBoard's own `jpaAttributeDao` bean — see the Entity attributes section. | | `iotdb.attributes.cluster_mode` | _(empty)_ | Required when `database.attributes.type=iotdb-table`. Must be `sticky-routing` (per-identity writes pinned to one node) or `disabled` (single-node / acknowledged best-effort); any other value (including the empty default) fails construction fast, because the attribute write path converges only within a single JVM. | | `iotdb.ts_latest.cluster_mode` | _(empty)_ | Required when `database.ts_latest.type=iotdb-table` (the latest-overlay DAO is active). Must be `sticky-routing` (per-identity latest writes pinned to one node) or `disabled` (single-node / acknowledged best-effort); any other value (including the empty default) fails construction fast, because the latest-overlay write path converges only within a single JVM. This is the symmetric acknowledgement to `iotdb.attributes.cluster_mode`. | | `iotdb.attributes.executor.threads` | `4` | Worker-thread count for the attribute DAO's bounded IO executor. Sized independently of `iotdb.ts.read.*` so the attribute path's concurrency can be tuned on its own; the default matches `iotdb.ts.read`. | @@ -380,8 +389,10 @@ behind its own `database.ts_latest.type=iotdb-table` selector. Physical retention is a table property the operator sets on the schema; see Retention / TTL above. -`IoTDBTableAttributesDao` is **inert by default** and activated only by the independent -`database.attributes.type=iotdb-table` opt-in (see the Entity attributes section -above and its Phase-1 limitations). In a real Phase-1 deployment the selector is -unset, so the attribute DAO never activates and attributes stay in the host -entity database. +`IoTDBTableAttributesDao` is **inert by default** and activated only by the +`database.attributes.type=iotdb-table` opt-in, which is separate from the two +timeseries selectors (see the Entity attributes section above and its Phase-1 +limitations). While the selector is unset — the default posture — the attribute +DAO never activates and attributes stay in the host entity database. Setting it +has the module withdraw ThingsBoard's own attributes bean, which is why that +section describes the matching rule and its boundary. diff --git a/iotdb-thingsboard-table/docs/migration-guide.md b/iotdb-thingsboard-table/docs/migration-guide.md index 344ed91..a16b755 100644 --- a/iotdb-thingsboard-table/docs/migration-guide.md +++ b/iotdb-thingsboard-table/docs/migration-guide.md @@ -319,12 +319,25 @@ Each route is independently activated and guarded: - **Independent activation.** Enabling telemetry does not enable attributes, and vice versa. You can route telemetry + latest to IoTDB while attributes stay in the host entity database (the default Phase-1 posture). -- **Fail-fast conflict guard.** When a route is enabled but a conflicting - non-IoTDB host DAO bean of the same SPI type is also present, startup fails - fast with a clear message rather than silently shadowing one DAO with another. - The historical (`TimeseriesDao`), latest (`TimeseriesLatestDao`), and attribute - (`AttributesDao`) routes each have their own guard. Make sure the host backend - for a route is removed/disabled when you point that route at IoTDB. +- **Conflict guards, and they are not all fail-fast.** The historical + (`TimeseriesDao`), latest (`TimeseriesLatestDao`), and attribute + (`AttributesDao`) routes each have their own guard, but they resolve the + conflict differently. + + The timeseries and latest guards fail startup when a conflicting non-IoTDB host + DAO of the same SPI type is present, rather than silently shadowing one DAO with + another. Remove or disable the host backend for that route before pointing it at + IoTDB. + + The attribute guard cannot ask for that, because ThingsBoard registers + `JpaAttributeDao` unconditionally and no configuration stands it down. So when + `database.attributes.type=iotdb-table` is set, the module withdraws that one bean + definition itself and logs a WARN naming it. The match is on both the bean name + and the exact fully-qualified class name, so nothing else is ever removed: any + other competing `AttributesDao` visible to the guard when it runs fails startup + untouched. Definitions registered after it, supplied by a `FactoryBean` that + does not report its type until initialisation, or inherited from a parent + context are outside its reach. ### Rollback @@ -370,10 +383,12 @@ module `README.md` for the authoritative list): with table-wide IoTDB TTL; the module uses it only for ThingsBoard's storage-accounting, never as a physical-retention directive. Set physical retention on the table (Step 5). -- **The attributes route is a stretch / Phase-2 opt-in.** No shipped ThingsBoard - release exposes a `database.attributes.type` selector yet (open question, - tracked upstream), so in a real Phase-1 deployment the selector is unset and - attributes stay in the host entity database. When activated, `save` is a +- **The attributes route is a stretch / Phase-2 opt-in.** `database.attributes.type` + is a selector this module supplies rather than one ThingsBoard offers; leaving it + unset is the default posture, and while unset attributes stay in the host entity + database. Setting it makes the module withdraw ThingsBoard's own attributes bean + — see the conflict-guard bullet above for the matching rule and its boundary. + When activated, `save` is a non-atomic tag-only delete-then-insert under a per-identity in-JVM lock that converges only within one JVM; `findNextBatch` is unsupported (`UnsupportedOperationException`), and `findAllKeysByDeviceProfileId` with a diff --git a/iotdb-thingsboard-table/docs/user-guide.md b/iotdb-thingsboard-table/docs/user-guide.md index 2c19743..b96e57e 100644 --- a/iotdb-thingsboard-table/docs/user-guide.md +++ b/iotdb-thingsboard-table/docs/user-guide.md @@ -76,22 +76,56 @@ Notes: timeseries selectors are required as well because the latest value is derived from the `telemetry` table that only the IoTDB writer populates — the latest path can never activate without that writer. -- **Attributes** is an opt-in stretch feature. No shipped - ThingsBoard release exposes a `database.attributes.type` selector yet (open - question Q6 / ThingsBoard Discussion #15296), so a real deployment normally - leaves it unset and attributes keep flowing to the host entity database. When - it is activated, `iotdb.attributes.cluster-mode` must also be set or the DAO - fails fast at startup (see [§6](#6-configuration-reference)). +- **Attributes** is an opt-in stretch feature, and `database.attributes.type` is + a selector this module supplies rather than one ThingsBoard offers. ThingsBoard + switches its timeseries DAOs by configuration but has no equivalent for + attributes: `JpaAttributeDao` is an unconditional `@Component` (verified at + v4.3.1.2), so nothing in `thingsboard.yml` can stand it down. Setting this + selector therefore has the module withdraw that one bean at startup, logging a + WARN that names it. Leaving the selector unset is the default posture and + attributes keep flowing to the host entity database. When it is activated, + `iotdb.attributes.cluster-mode` must also be set or the DAO fails fast at + startup (see [§6](#6-configuration-reference)). Open question Q6 / ThingsBoard + Discussion #15296 tracks a native selector; if one ships, this module should + use it instead. When activated, the DAOs share a single module-owned IoTDB table session pool. ### Conflict guards -When a selector is on, the module fails startup fast if a conflicting non-IoTDB -DAO bean of the same SPI type is also present (for example, another -`TimeseriesDao` while `database.ts.type=iotdb-table`). This is deliberate: it -prevents the module from silently shadowing, or being shadowed by, a different -backend. Remove the conflicting backend or unset the IoTDB selector. +When a selector is on, the module refuses to share its SPI slot with another +backend. The timeseries and latest guards do this by failing startup: if a +conflicting non-IoTDB `TimeseriesDao` or `TimeseriesLatestDao` bean is present +while the matching selector is set, startup stops with a message naming it. +Remove the conflicting backend or unset the IoTDB selector. + +The attributes guard behaves differently, because the conflict it faces is not +one you can resolve from a configuration file. ThingsBoard registers +`JpaAttributeDao` unconditionally, so "remove the conflicting backend" is not +advice an operator can act on. When `database.attributes.type=iotdb-table` is +set, the module therefore withdraws that one bean definition itself and logs the +line below. `AttributesDaoConflictGuardTest` asserts that the module emits it; +it was also observed in a live run against `thingsboard/tb-node:4.3.1.2` on +2026-08-20: + +```text +WARN Removed ThingsBoard bean 'jpaAttributeDao' + (org.thingsboard.server.dao.sql.attributes.JpaAttributeDao) + because database.attributes.type=iotdb-table selects the IoTDB attributes + backend; ... +``` + +The withdrawal is narrow on purpose. It matches on both the bean name and the +exact fully-qualified class name, so **only** ThingsBoard's own component is +removed. Any other +competing `AttributesDao` — a third-party backend, a decorator, or a subclass of +this module's DAO under a different bean name — is left in place and startup +fails instead, naming it. A bean your application registered deliberately is not +the module's to delete. + +The guard reads the bean definitions present when it runs. A definition +registered later, supplied by a `FactoryBean` that does not report its type until +initialisation, or inherited from a parent context is outside its reach. ## 4. The three DAOs diff --git a/iotdb-thingsboard-table/pom.xml b/iotdb-thingsboard-table/pom.xml index 43b65a2..63e8f7b 100644 --- a/iotdb-thingsboard-table/pom.xml +++ b/iotdb-thingsboard-table/pom.xml @@ -99,6 +99,24 @@ spring-context provided + + + ch.qos.logback + logback-classic + test + + + + ch.qos.logback + logback-core + test + org.springframework.boot spring-boot-autoconfigure @@ -130,14 +148,23 @@ test - + org.apache.iotdb iotdb-thrift-commons - test + runtime org.springframework.boot @@ -264,6 +291,8 @@ test + + @{argLine} -Xmx1024m -javaagent:${org.mockito:mockito-core:jar} **/*Test.java @@ -278,6 +307,12 @@ org.apache.maven.plugins maven-dependency-plugin + + mockito-agent-path + + properties + + check-dependencies diff --git a/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesDao.java b/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesDao.java index fbd90d9..baf3574 100644 --- a/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesDao.java +++ b/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesDao.java @@ -55,14 +55,15 @@ /** * Entity-attribute DAO for the IoTDB Table Mode backend. * - *

Spring activation: {@code database.attributes.type=iotdb-table}. NOTE: this activation - * property is the Phase-1 selector pending upstream ThingsBoard confirmation; upstream ThingsBoard - * does not yet expose an {@code AttributesDao} selector, so the DAO is inert by default (no - * real Phase-1 deployment sets {@code database.attributes.type}, so the {@link - * IoTDBTableAttributesEnabledCondition} stays false and the bean is never instantiated). The - * selector is independent of {@code database.ts.type} / {@code database.ts_latest.type} (the - * attribute DAO routes separately); if upstream resolves to a different property, the condition is - * updated. Phase-1 attributes stay in the host entity DB. + *

Spring activation: {@code database.attributes.type=iotdb-table}. This is a selector this + * module supplies rather than one upstream ThingsBoard offers -- upstream exposes no {@code + * AttributesDao} selector of its own -- so the DAO is inert by default: while the property + * is unset, {@link IoTDBTableAttributesEnabledCondition} stays false, the bean is never + * instantiated, and attributes stay in the host entity DB. Setting it makes {@code + * AttributesDaoConflictGuard} withdraw ThingsBoard's own attributes bean; see that guard's javadoc + * for the matching rule and the boundary of what it can see. The selector is independent of {@code + * database.ts.type} / {@code database.ts_latest.type} (the attribute DAO routes separately); if + * upstream ever exposes a native selector, this module should use it instead. * *

This DAO is wired as an explicit {@code @Bean} in {@link IoTDBTableConfiguration} (guarded by * the activation property) rather than via component scanning, so the {@code ITableSessionPool} diff --git a/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesEnabledCondition.java b/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesEnabledCondition.java index 50dc515..f06c137 100644 --- a/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesEnabledCondition.java +++ b/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableAttributesEnabledCondition.java @@ -28,15 +28,16 @@ * *

This selector is INDEPENDENT of {@code database.ts.type} / {@code database.ts_latest.type}: * the attribute DAO routes separately from the time-series DAOs (a piggy-back on the timeseries - * selector was deliberately rejected). Because upstream ThingsBoard does not expose an {@code - * AttributesDao} selector yet, no real Phase-1 deployment sets {@code database.attributes.type}; - * the property is therefore absent in practice, this condition returns false, the attribute bean is - * never instantiated, and attributes keep flowing to the host entity-DB {@code AttributesDao}. The - * DAO is thus inert by default and only activates when an operator opts in explicitly. + * selector was deliberately rejected). Upstream ThingsBoard exposes no {@code AttributesDao} + * selector of its own, so {@code database.attributes.type} is one this module supplies. Leaving it + * unset is the default posture: this condition returns false, the attribute bean is never + * instantiated, and attributes keep flowing to the host entity-DB {@code AttributesDao}. The DAO is + * inert by default and activates only when an operator opts in explicitly -- at which point {@code + * AttributesDaoConflictGuard} withdraws ThingsBoard's own attributes bean. */ final class IoTDBTableAttributesEnabledCondition implements Condition { - private static final String SELECTOR_PROPERTY = "database.attributes.type"; - private static final String SELECTOR_VALUE = "iotdb-table"; + static final String SELECTOR_PROPERTY = "database.attributes.type"; + static final String SELECTOR_VALUE = "iotdb-table"; @Override public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { diff --git a/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableConfiguration.java b/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableConfiguration.java index 6384400..407b539 100644 --- a/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableConfiguration.java +++ b/iotdb-thingsboard-table/src/main/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableConfiguration.java @@ -27,6 +27,7 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -36,10 +37,16 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; import org.springframework.core.ResolvableType; import org.springframework.util.ClassUtils; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; /** * Spring Boot auto-configuration entry point for the IoTDB Table Mode backend. @@ -76,6 +83,12 @@ public class IoTDBTableConfiguration { "org.thingsboard.server.dao.timeseries.TimeseriesLatestDao"; static final String ATTRIBUTES_DAO_CLASS_NAME = "org.thingsboard.server.dao.attributes.AttributesDao"; + // ThingsBoard's own attributes component. Verified at v4.3.1.2 (tag c37fb509): + // dao/src/main/java/org/thingsboard/server/dao/sql/attributes/JpaAttributeDao.java:58-61 is a + // bare @Component on this class, so Spring's default name is the uncapitalised simple name. + static final String JPA_ATTRIBUTE_DAO_CLASS_NAME = + "org.thingsboard.server.dao.sql.attributes.JpaAttributeDao"; + static final String JPA_ATTRIBUTE_DAO_BEAN_NAME = "jpaAttributeDao"; @Configuration(proxyBeanMethods = false) @ConditionalOnClass(name = TIMESERIES_DAO_CLASS_NAME) @@ -211,10 +224,11 @@ IoTDBTableSchemaBootstrap latestSchemaBootstrap( * a separate inner configuration from {@link EnabledRawOnlyConfiguration} because the attribute * DAO routes separately from the time-series DAOs: it must be able to activate on its own * (attributes selector set, ts selectors unset) and must stay inert when no attributes selector - * is present. Because no shipped ThingsBoard release exposes {@code database.attributes.type}, - * the default Phase-1 deployment leaves it unset, this configuration is skipped, no session pool - * or attribute bean is created, and attributes keep flowing to the host entity-DB {@code - * AttributesDao} (inert by default). + * is present. Leaving the selector unset is the default posture: this configuration is skipped, + * no session pool or attribute bean is created, and attributes keep flowing to the host entity-DB + * {@code AttributesDao} (inert by default). {@code database.attributes.type} is a selector this + * module supplies rather than one ThingsBoard offers -- see {@link #attributesDaoConflictGuard()} + * for what setting it does to the host's own attributes bean. * *

The session pool / schema bootstrap beans here reuse the same bean name as {@link * EnabledRawOnlyConfiguration} and carry {@code @ConditionalOnMissingBean(name=...)}, so when @@ -234,10 +248,25 @@ ITableSessionPool tableSessionPool(IoTDBTableConfig config) { } /** - * Fails startup before any IoTDB pool/bootstrap singleton is created if the explicit IoTDB - * attribute backend selection conflicts with a host-provided {@code AttributesDao}, mirroring - * {@code timeseriesDaoConflictGuard()} so the attribute path does not silently shadow a - * different backend. + * Resolves the attributes-backend conflict before any IoTDB pool/bootstrap singleton is + * created. Unlike its timeseries siblings this guard does not only fail: ThingsBoard switches + * its timeseries DAOs by configuration but offers no equivalent for attributes, so when the + * IoTDB attributes backend is selected the guard withdraws ThingsBoard's own {@code + * jpaAttributeDao} bean definition and logs a WARN naming it. + * + *

Withdrawal is deliberately narrow. It applies to exactly one bean, matched on both the + * bean name {@code jpaAttributeDao} and the resolved type {@code + * org.thingsboard.server.dao.sql.attributes.JpaAttributeDao}. Any other competing + * {@code AttributesDao} — a third-party backend, a decorator, a subclass of this module's own + * DAO — fails startup untouched, because a bean the operator registered deliberately is not + * ours to delete. + * + *

Scope of the guarantee. Candidates are discovered from a single {@code + * getBeanNamesForType(type, true, false)} snapshot, which does not initialise FactoryBeans and + * does not consult a parent factory. What this guard promises is therefore bounded to the + * definitions visible in this bean factory at the moment it runs: a definition registered by a + * later post-processor, produced by an opaque {@code FactoryBean} whose {@code getObjectType()} + * is null until initialisation, or inherited from an ancestor context is outside it. */ @Bean static BeanFactoryPostProcessor attributesDaoConflictGuard() { @@ -250,9 +279,17 @@ static BeanFactoryPostProcessor attributesDaoConflictGuard() { * classes while evaluating auto-configuration metadata, and the {@code @Bean} destroy method * drains the DAO's IO executor on shutdown. */ + // NOTE: deliberately NOT @ConditionalOnMissingBean(type = ATTRIBUTES_DAO_CLASS_NAME). + // At ThingsBoard v4.3.1.2 JpaAttributeDao is an unconditional @Component, and that condition is + // evaluated while configuration classes are parsed -- strictly BEFORE + // attributesDaoConflictGuard() runs. Keeping it meant this bean was skipped on that build, so + // the selector could never take effect. The guard now resolves the conflict instead: it + // withdraws ThingsBoard's own attributes bean, or refuses to start if it finds any other + // competing AttributesDao. Within the definitions visible to the guard when it runs, that + // leaves exactly one -- this one. It is not a guarantee about definitions the guard cannot + // see; see attributesDaoConflictGuard()'s javadoc for that boundary. @Bean(name = IOTDB_TABLE_ATTRIBUTES_DAO_BEAN_NAME, destroyMethod = "destroy") @ConditionalOnBean(name = IOTDB_TABLE_SESSION_POOL_BEAN_NAME) - @ConditionalOnMissingBean(type = ATTRIBUTES_DAO_CLASS_NAME) IoTDBTableAttributesDao ioTDBTableAttributesDao( @Qualifier(IOTDB_TABLE_SESSION_POOL_BEAN_NAME) ITableSessionPool tableSessionPool, IoTDBTableConfig config) { @@ -407,33 +444,165 @@ private static Class resolveTimeseriesLatestDaoClass( } } - private static final class AttributesDaoConflictGuard implements BeanFactoryPostProcessor { + private static final class AttributesDaoConflictGuard + implements BeanFactoryPostProcessor, PriorityOrdered { + + // This guard MUTATES bean definitions; its throw-only siblings do not. PriorityOrdered with + // HIGHEST_PRECEDENCE puts it ahead of other regular BeanFactoryPostProcessors, which is what + // keeps the withdrawal ahead of anything that would resolve AttributesDao through one. It does + // NOT order this guard against BeanDefinitionRegistryPostProcessors, which run as a separate + // earlier phase -- a definition registered there is simply part of the snapshot this guard + // reads, while one registered by a LATER post-processor is outside what it can see at all. + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } + @Override public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { + // PHASE 0 -- the only check that depends on no candidate, so it is answered once. Testing + // this per-candidate made the failure message depend on iteration order. + if (!(beanFactory instanceof BeanDefinitionRegistry registry)) { + throw new IllegalStateException( + "database.attributes.type=iotdb-table, but this bean factory is not a " + + "BeanDefinitionRegistry, so ThingsBoard's competing attributes bean cannot be " + + "withdrawn; unset the IoTDB attributes selector"); + } Class attributesDaoType = resolveAttributesDaoClass(beanFactory); + + // PHASE 1 -- read-only. Nothing below this point mutates until every reason to stop has + // been evaluated. + // + // (a) OUR bean is found by DIRECT NAME LOOKUP, not by filtering the type snapshot. + // Assignability is not identity: a user subclass of IoTDBTableAttributesDao is + // somebody else's bean that happens to extend ours. Looking the name up directly also + // catches a bean that took our name while implementing something else entirely -- + // that bean never enters the AttributesDao snapshot at all. + boolean ourDefinitionPresent = + beanFactory.containsBeanDefinition(IOTDB_TABLE_ATTRIBUTES_DAO_BEAN_NAME); + Class ourType = + ourDefinitionPresent + ? resolveBeanType(beanFactory, IOTDB_TABLE_ATTRIBUTES_DAO_BEAN_NAME) + : null; + + // (b) every OTHER visible AttributesDao candidate falls into one of two classes: + // + // KNOWN_TARGET ThingsBoard's own attributes component, matched CONJUNCTIVELY on the + // default component name AND the exact resolved class name. This is the + // single bean the explicit database.attributes.type=iotdb-table selector + // asks this module to replace, and the only one the documentation + // names. Verified at ThingsBoard v4.3.1.2 (tag c37fb509): + // JpaAttributeDao is a bare @Component on that class, hence that + // name. + // UNKNOWN everything else -- a subclass, a decorator, a third-party backend, or a + // right-name/wrong-type imposter. Deleting a bean an operator wired on + // purpose is worse than the ambiguity it would prevent, so these keep the + // original fail-fast semantics, and that advice is now actionable: the + // bean belongs to the application, which can remove it. + // + // Discovery is getBeanNamesForType(type, true, false): a one-time snapshot that does not + // initialise FactoryBeans and does not consult a parent factory. Definitions registered + // after this post-processor, produced by an opaque FactoryBean whose getObjectType() is + // null until initialisation, or inherited from an ancestor context are outside what this + // guard can see -- and therefore outside what it promises. + // At most ONE bean can ever be the known target: the match is conjunctive on a fixed bean + // name, and bean names are unique within a factory. A collection here would imply a + // generality that cannot occur -- the same objection that removed an unreachable + // "more than one of ours" branch from an earlier draft. + String knownTargetName = null; + Class knownTargetType = null; + Map> unknown = new LinkedHashMap<>(); + List unresolvable = new ArrayList<>(); + for (String beanName : beanFactory.getBeanNamesForType(attributesDaoType, true, false)) { - if (!isIoTDBAttributesDaoBean(beanFactory, beanName)) { - throw new IllegalStateException( - "database.attributes.type=iotdb-table, but a non-IoTDB AttributesDao bean '" - + beanName - + "' is present; remove it or unset the IoTDB attributes selector"); + if (IOTDB_TABLE_ATTRIBUTES_DAO_BEAN_NAME.equals(beanName)) { + continue; + } + Class beanType = resolveBeanType(beanFactory, beanName); + if (beanType == null) { + unresolvable.add(beanName); + } else if (JPA_ATTRIBUTE_DAO_BEAN_NAME.equals(beanName) + && JPA_ATTRIBUTE_DAO_CLASS_NAME.equals(beanType.getName())) { + knownTargetName = beanName; + knownTargetType = beanType; + } else { + unknown.put(beanName, beanType); } } - } - private static boolean isIoTDBAttributesDaoBean( - ConfigurableListableBeanFactory beanFactory, String beanName) { - Class beanType = resolveBeanType(beanFactory, beanName); - if (beanType == null) { + // An earlier version checked each candidate's removability inside the mutation loop, so it + // could withdraw candidate one and then throw on candidate two, leaving a half-mutated + // context that the surrounding comment claimed was impossible. + if (!unresolvable.isEmpty()) { throw new IllegalStateException( - "database.attributes.type=iotdb-table, but AttributesDao bean '" - + beanName - + "' has no resolvable type; expose a concrete IoTDBTableAttributesDao type or " - + "remove the bean"); + "database.attributes.type=iotdb-table, but AttributesDao bean(s) " + + unresolvable + + " have no resolvable type and cannot be classified; expose a concrete type or " + + "remove the bean(s). Nothing was withdrawn"); } - // beanType is guaranteed non-null here (the null case throws above). - return IoTDBTableAttributesDao.class.isAssignableFrom(beanType); + if (!ourDefinitionPresent) { + throw new IllegalStateException( + "database.attributes.type=iotdb-table, but the IoTDB Table Mode attributes DAO bean '" + + IOTDB_TABLE_ATTRIBUTES_DAO_BEAN_NAME + + "' did not register (check the session pool bean and the with-thingsboard " + + "build); NOT withdrawing competing AttributesDao bean(s) " + + competing(knownTargetName, unknown.keySet())); + } + if (ourType == null || !IoTDBTableAttributesDao.class.isAssignableFrom(ourType)) { + throw new IllegalStateException( + "database.attributes.type=iotdb-table, but bean '" + + IOTDB_TABLE_ATTRIBUTES_DAO_BEAN_NAME + + "' is " + + (ourType == null ? "of no resolvable type" : "a " + ourType.getName()) + + " rather than an IoTDBTableAttributesDao; something else holds this module's " + + "bean name. Nothing was withdrawn"); + } + if (!unknown.isEmpty()) { + throw new IllegalStateException( + "database.attributes.type=iotdb-table selects the IoTDB attributes backend, but " + + "bean(s) " + + unknown + + " also implement AttributesDao. This module withdraws only ThingsBoard's own '" + + JPA_ATTRIBUTE_DAO_BEAN_NAME + + "' (" + + JPA_ATTRIBUTE_DAO_CLASS_NAME + + "); it will not remove a bean your application registered. Remove the " + + "conflicting bean(s) or unset the selector. Nothing was withdrawn"); + } + if (knownTargetName != null && !registry.containsBeanDefinition(knownTargetName)) { + throw new IllegalStateException( + "database.attributes.type=iotdb-table, but ThingsBoard's attributes bean '" + + knownTargetName + + "' has no bean definition in this registry (it was most likely supplied as a " + + "pre-built singleton) and cannot be withdrawn; unset the IoTDB attributes " + + "selector. Nothing was withdrawn"); + } + + // PHASE 2 -- the single mutation. The bean removed here has been established to be + // ThingsBoard's own component and to have a removable definition, so the WARN's wording is + // true by construction rather than by assumption. + if (knownTargetName != null) { + registry.removeBeanDefinition(knownTargetName); + log.warn( + "Removed ThingsBoard bean '{}' ({}) because {}={} selects the IoTDB attributes " + + "backend; ThingsBoard provides no configuration switch for attributes, so the " + + "conflicting bean is deregistered rather than left to conflict.", + knownTargetName, + knownTargetType.getName(), + IoTDBTableAttributesEnabledCondition.SELECTOR_PROPERTY, + IoTDBTableAttributesEnabledCondition.SELECTOR_VALUE); + } + } + + /** Names every bean that competes for the AttributesDao role, for a refusal message. */ + private static List competing(String knownTargetName, Collection unknown) { + List all = new ArrayList<>(); + if (knownTargetName != null) { + all.add(knownTargetName); + } + all.addAll(unknown); + return all; } private static Class resolveBeanType( diff --git a/iotdb-thingsboard-table/src/provided/java/org/thingsboard/server/dao/sql/attributes/JpaAttributeDao.java b/iotdb-thingsboard-table/src/provided/java/org/thingsboard/server/dao/sql/attributes/JpaAttributeDao.java new file mode 100644 index 0000000..a2ceede --- /dev/null +++ b/iotdb-thingsboard-table/src/provided/java/org/thingsboard/server/dao/sql/attributes/JpaAttributeDao.java @@ -0,0 +1,162 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Compile-only ThingsBoard stub (Strategy F). Verified against ThingsBoard v4.3.1.2 +// (commit c37fb509). +package org.thingsboard.server.dao.sql.attributes; + +import com.google.common.util.concurrent.ListenableFuture; +import org.apache.commons.lang3.tuple.Pair; +import org.thingsboard.server.common.data.AttributeScope; +import org.thingsboard.server.common.data.id.DeviceProfileId; +import org.thingsboard.server.common.data.id.EntityId; +import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.common.data.kv.AttributeKvEntry; +import org.thingsboard.server.common.data.util.TbPair; +import org.thingsboard.server.dao.attributes.AttributesDao; +import org.thingsboard.server.dao.model.sql.AttributeKvEntity; + +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +/** + * Compile-only ThingsBoard surface stub (Strategy F) for {@code + * org.thingsboard.server.dao.sql.attributes.JpaAttributeDao}, ThingsBoard's built-in JPA + * entity-attribute DAO. Excluded from the built jar, so it can never shadow the real class on a + * deployment classpath. + * + *

Only this class's fully-qualified NAME carries meaning here. {@code + * AttributesDaoConflictGuard} identifies the one host bean it is authorised to withdraw by matching + * bean name {@code jpaAttributeDao} against resolved type {@code + * org.thingsboard.server.dao.sql.attributes.JpaAttributeDao}; without a class of that exact name on + * the test classpath that branch could not be exercised at all. The method bodies are never + * executed and the guard never instantiates the bean -- it runs as a {@code + * BeanFactoryPostProcessor}, before any bean is created. + * + *

Two divergences from the real class, deliberate and harmless for the above purpose: the real + * one is a {@code @Component} extending {@code JpaAbstractDaoListeningExecutorService}, and it + * implements these methods against JPA repositories. This stub declares neither the annotation nor + * the superclass, because the guard reads a bean definition's type and never the class's + * annotations or hierarchy. + * + *

The name itself is not proven by any test in this module: ThingsBoard's dao artifact is not on + * Maven Central, so the string was taken from ThingsBoard's own source at v4.3.1.2, {@code + * dao/src/main/java/org/thingsboard/server/dao/sql/attributes/JpaAttributeDao.java:58-61}, read + * independently twice. If ThingsBoard ever renames or repackages that class, this module's + * attributes selector fails closed -- the bean becomes UNKNOWN and startup stops with a message + * naming it -- rather than silently withdrawing the wrong bean. + */ +public class JpaAttributeDao implements AttributesDao { + + private static UnsupportedOperationException notExecutable() { + return new UnsupportedOperationException( + "compile-only ThingsBoard stub; the real implementation is supplied by the ThingsBoard " + + "runtime classpath"); + } + + @Override + public Optional find( + TenantId tenantId, EntityId entityId, AttributeScope attributeScope, String attributeKey) { + throw notExecutable(); + } + + @Override + public List find( + TenantId tenantId, + EntityId entityId, + AttributeScope attributeScope, + Collection attributeKey) { + throw notExecutable(); + } + + @Override + public List findAll( + TenantId tenantId, EntityId entityId, AttributeScope attributeScope) { + throw notExecutable(); + } + + @Override + public ListenableFuture save( + TenantId tenantId, + EntityId entityId, + AttributeScope attributeScope, + AttributeKvEntry attribute) { + throw notExecutable(); + } + + @Override + public List> removeAll( + TenantId tenantId, EntityId entityId, AttributeScope attributeScope, List keys) { + throw notExecutable(); + } + + @Override + public List>> removeAllWithVersions( + TenantId tenantId, EntityId entityId, AttributeScope attributeScope, List keys) { + throw notExecutable(); + } + + @Override + public List findNextBatch( + UUID entityId, int attributeType, int attributeKey, int batchSize) { + throw notExecutable(); + } + + @Override + public List findAllKeysByDeviceProfileId( + TenantId tenantId, DeviceProfileId deviceProfileId) { + throw notExecutable(); + } + + @Override + public List findAllKeysByEntityIds(TenantId tenantId, List entityIds) { + throw notExecutable(); + } + + @Override + public List findAllKeysByEntityIdsAndScope( + TenantId tenantId, List entityIds, AttributeScope scope) { + throw notExecutable(); + } + + @Override + public ListenableFuture> findAllKeysByEntityIdsAndScopeAsync( + TenantId tenantId, List entityIds, AttributeScope scope) { + throw notExecutable(); + } + + @Override + public List findLatestByEntityIdsAndScope( + TenantId tenantId, List entityIds, AttributeScope scope) { + throw notExecutable(); + } + + @Override + public ListenableFuture> findLatestByEntityIdsAndScopeAsync( + TenantId tenantId, List entityIds, AttributeScope scope) { + throw notExecutable(); + } + + @Override + public List> removeAllByEntityId( + TenantId tenantId, EntityId entityId) { + throw notExecutable(); + } +} diff --git a/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/AttributesDaoConflictGuardTest.java b/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/AttributesDaoConflictGuardTest.java new file mode 100644 index 0000000..e802774 --- /dev/null +++ b/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/AttributesDaoConflictGuardTest.java @@ -0,0 +1,444 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.iotdb.extras.thingsboard.table; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.config.BeanFactoryPostProcessor; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; +import org.thingsboard.server.dao.attributes.AttributesDao; +import org.thingsboard.server.dao.sql.attributes.JpaAttributeDao; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +/** + * Behavioural cover for {@code AttributesDaoConflictGuard}, the post-processor that makes the IoTDB + * attributes selector usable on a stock ThingsBoard. + * + *

ThingsBoard registers {@code JpaAttributeDao} as an unconditional {@code @Component} and + * exposes no attributes-backend switch of its own, so selecting IoTDB has to withdraw the competing + * definition. Withdrawing is only safe while a replacement of the same type is registered: + * otherwise the context ends up with zero {@code AttributesDao} beans, which is a startup outage + * rather than a degraded mode. + * + *

This class exists because the guard previously had none. A change to its behaviour passed 190 + * green unit tests without a single failure, which is precisely the gap a guard of this kind must + * not have — it decides whether the host application starts at all. + * + *

The guard never instantiates a bean; it reads definitions and types only. + * + *

Two different stand-ins, and the difference is the point. The guard identifies the one + * bean it may withdraw conjunctively: bean name {@code jpaAttributeDao} AND resolved type {@code + * org.thingsboard.server.dao.sql.attributes.JpaAttributeDao}. So the host's DAO is represented by + * the compile-only stub of that exact class ({@link JpaAttributeDao}, Strategy F, excluded from the + * built jar), and a Mockito-derived {@link AttributesDao} now stands for something else entirely -- + * a third-party backend or a user's own bean, which the guard must refuse to touch. An earlier + * revision used the Mockito class for ThingsBoard's DAO; under a name-only rule that was + * indistinguishable, and the indistinguishability was the defect. + * + *

What these tests do NOT establish: that {@code + * org.thingsboard.server.dao.sql.attributes.JpaAttributeDao} is the right string. ThingsBoard's dao + * artifact is not on Maven Central, so that name comes from reading ThingsBoard's own source at + * v4.3.1.2, not from anything asserted here. + */ +class AttributesDaoConflictGuardTest { + + private static final String IOTDB_DAO_BEAN = "ioTDBTableAttributesDao"; + private static final String HOST_DAO_BEAN = "jpaAttributeDao"; + + private static final String PEER_DAO_BEAN = "someOtherIoTDBAttributesDao"; + private static final String THIRD_PARTY_DAO_BEAN = "auditingAttributesDao"; + + /** ThingsBoard's own DAO: the compile-only stub carrying the real fully-qualified name. */ + private static final Class HOST_DAO_TYPE = JpaAttributeDao.class; + + /** + * An AttributesDao that is neither ours nor ThingsBoard's -- a third-party backend or a bean the + * operator wrote. The guard has no standing to delete this and must fail loudly instead. + */ + private static final Class THIRD_PARTY_DAO_TYPE = mock(AttributesDao.class).getClass(); + + private static BeanFactoryPostProcessor guard() { + return IoTDBTableConfiguration.EnabledAttributesConfiguration.attributesDaoConflictGuard(); + } + + private static DefaultListableBeanFactory factory(boolean withOurs, boolean withHost) { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + if (withOurs) { + beanFactory.registerBeanDefinition( + IOTDB_DAO_BEAN, new RootBeanDefinition(IoTDBTableAttributesDao.class)); + } + if (withHost) { + beanFactory.registerBeanDefinition(HOST_DAO_BEAN, new RootBeanDefinition(HOST_DAO_TYPE)); + } + return beanFactory; + } + + /** T1 — the competing definition is withdrawn and ours is left strictly alone. */ + @Test + void withdrawsTheHostDaoAndLeavesOursUntouched() { + DefaultListableBeanFactory beanFactory = factory(true, true); + + guard().postProcessBeanFactory(beanFactory); + + assertFalse( + beanFactory.containsBeanDefinition(HOST_DAO_BEAN), "competing definition withdrawn"); + assertTrue(beanFactory.containsBeanDefinition(IOTDB_DAO_BEAN), "our definition untouched"); + assertEquals( + 1, + beanFactory.getBeanNamesForType(AttributesDao.class, true, false).length, + "exactly one AttributesDao candidate remains"); + } + + /** + * T3 — the zero-bean invariant, and the case that actually occurred in production this morning: + * our DAO bean was skipped by a condition, so withdrawing ThingsBoard's would have left the + * context with no AttributesDao at all. The guard must refuse BEFORE mutating anything. + */ + @Test + void refusesToWithdrawWhenOurReplacementDidNotRegister() { + DefaultListableBeanFactory beanFactory = factory(false, true); + + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue( + thrown.getMessage().contains("did not register"), + "states that our DAO did not register: " + thrown.getMessage()); + assertTrue( + thrown.getMessage().contains(HOST_DAO_BEAN), + "names the bean it declined to withdraw: " + thrown.getMessage()); + assertTrue( + beanFactory.containsBeanDefinition(HOST_DAO_BEAN), + "nothing withdrawn: the throw precedes every mutation"); + } + + /** With no competing bean at all the guard is a no-op, not a failure. */ + @Test + void isANoOpWhenOnlyOurDaoIsRegistered() { + DefaultListableBeanFactory beanFactory = factory(true, false); + + guard().postProcessBeanFactory(beanFactory); + + assertTrue(beanFactory.containsBeanDefinition(IOTDB_DAO_BEAN), "our definition survives"); + assertEquals( + 1, + beanFactory.getBeanNamesForType(AttributesDao.class, true, false).length, + "exactly one AttributesDao candidate remains"); + } + + /** + * A definition whose class cannot be resolved is not treated as an {@code AttributesDao} + * candidate at all, so the guard neither withdraws it nor fails. + * + *

This documents why the guard's own unresolvable-type branch is defensive rather than + * reachable from here: {@code getBeanNamesForType} cannot match a definition whose class it + * cannot load, so such a bean never enters the partitioning loop. The test asserts the mechanism, + * not just the outcome — if a future Spring version starts listing unresolvable definitions, the + * candidate-count assertion fails and this comment stops being true. + */ + @Test + void anUnresolvableDefinitionIsNotAnAttributesDaoCandidate() { + DefaultListableBeanFactory beanFactory = factory(true, false); + RootBeanDefinition unresolvable = new RootBeanDefinition(); + unresolvable.setBeanClassName("org.thingsboard.server.dao.attributes.NoSuchAttributeDao"); + beanFactory.registerBeanDefinition(HOST_DAO_BEAN, unresolvable); + + assertEquals( + 1, + beanFactory.getBeanNamesForType(AttributesDao.class, true, false).length, + "the unresolvable definition is not listed as an AttributesDao candidate"); + + guard().postProcessBeanFactory(beanFactory); + + assertTrue( + beanFactory.containsBeanDefinition(HOST_DAO_BEAN), + "an unresolvable definition is left alone rather than withdrawn"); + assertTrue(beanFactory.containsBeanDefinition(IOTDB_DAO_BEAN), "our definition untouched"); + } + + /** + * ThingsBoard's DAO supplied as a pre-built singleton rather than a bean definition: visible to + * the type scan, but there is no definition to remove. The guard must detect that in its + * pre-flight and refuse, leaving the singleton in place. + * + *

No subclass or mock is involved: this is a genuine {@code DefaultListableBeanFactory} with a + * real {@code registerSingleton} call, which is how such a bean actually arrives. + */ + @Test + void throwsWhenTheHostDefinitionCannotBeWithdrawn() { + DefaultListableBeanFactory beanFactory = factory(true, false); + beanFactory.registerSingleton(HOST_DAO_BEAN, new JpaAttributeDao()); + + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue( + thrown.getMessage().contains("cannot be withdrawn"), + "message explains the definition could not be withdrawn: " + thrown.getMessage()); + assertTrue( + thrown.getMessage().contains(HOST_DAO_BEAN), + "message names the bean: " + thrown.getMessage()); + assertTrue( + beanFactory.containsSingleton(HOST_DAO_BEAN), + "the bean it could not withdraw is still there: the throw precedes every mutation"); + assertTrue(beanFactory.containsBeanDefinition(IOTDB_DAO_BEAN), "our definition untouched"); + } + + /** + * ThingsBoard's own attributes DAO for the context tests. + * + *

The declared return type is the concrete class, not {@code AttributesDao}, and that is + * load-bearing. The guard runs before any bean is created, so it reads the type a definition + * DECLARES, never the runtime class of an instance. ThingsBoard registers this bean by component + * scan, whose definition carries the concrete class; an interface-typed {@code @Bean} factory + * method would resolve only to {@code AttributesDao} and the guard would classify it as + * unrecognised and refuse to start -- correctly, since at that point nothing distinguishes it + * from a third-party backend. + */ + @Configuration(proxyBeanMethods = false) + static class HostAttributesDaoConfiguration { + @Bean(name = HOST_DAO_BEAN) + JpaAttributeDao jpaAttributeDao() { + return new JpaAttributeDao(); + } + } + + private static ApplicationContextRunner runner() { + return new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(IoTDBTableConfiguration.class)) + .withUserConfiguration(HostAttributesDaoConfiguration.class); + } + + /** + * With the selector unset the attributes path stays inert: the host's own DAO survives untouched + * and none of ours exist. This pins that the guard cannot fire without the property that + * justifies it. + */ + @Test + void withoutTheSelectorTheHostDaoSurvivesAndNoneOfOursExist() { + runner() + .withPropertyValues( + "iotdb.host=localhost", + "iotdb.port=6667", + "iotdb.username=root", + "iotdb.password=root", + "iotdb.schema.bootstrap=false") + .run( + context -> { + assertTrue(context.containsBean(HOST_DAO_BEAN), "host DAO untouched"); + assertFalse( + context.containsBean(IOTDB_DAO_BEAN), "our attributes DAO is not registered"); + }); + } + + /** + * With the selector set, the host's DAO is gone and exactly one AttributesDao remains, ours. This + * is also the regression test for the removed {@code @ConditionalOnMissingBean(type = + * AttributesDao)}, which previously skipped our bean on every stock ThingsBoard and left the + * context with no AttributesDao at all. + */ + @Test + void withTheSelectorOurDaoReplacesTheHostDao() { + runner() + .withPropertyValues( + "database.attributes.type=iotdb-table", + "iotdb.attributes.cluster_mode=disabled", + "iotdb.host=localhost", + "iotdb.port=6667", + "iotdb.username=root", + "iotdb.password=root", + "iotdb.schema.bootstrap=false") + .run( + context -> { + assertFalse(context.containsBean(HOST_DAO_BEAN), "host DAO withdrawn"); + assertEquals( + 1, + context.getBeanNamesForType(AttributesDao.class).length, + "exactly one AttributesDao remains"); + assertTrue(context.containsBean(IOTDB_DAO_BEAN), "and it is ours"); + }); + } + + /** + * The withdrawal must be visible in the log, because it is the only signal an operator gets that + * a bean from their own application was removed. The assertion covers all four facts a reader + * needs: which bean, its concrete class, and the property and value that caused it. + */ + @Test + void logsAWarnNamingTheBeanItsClassAndTheCausingProperty() { + Logger configurationLogger = (Logger) LoggerFactory.getLogger(IoTDBTableConfiguration.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + configurationLogger.addAppender(appender); + try { + guard().postProcessBeanFactory(factory(true, true)); + + ILoggingEvent event = + appender.list.stream() + .filter(e -> e.getFormattedMessage().contains("Removed ThingsBoard bean")) + .findFirst() + .orElseThrow(() -> new AssertionError("no withdrawal log line was emitted")); + + assertEquals(Level.WARN, event.getLevel(), "the withdrawal is logged at WARN"); + String message = event.getFormattedMessage(); + assertTrue(message.contains(HOST_DAO_BEAN), "names the bean: " + message); + assertTrue(message.contains(HOST_DAO_TYPE.getName()), "names its class: " + message); + assertTrue(message.contains("database.attributes.type"), "names the property: " + message); + assertTrue(message.contains("iotdb-table"), "names the value: " + message); + } finally { + configurationLogger.detachAppender(appender); + } + } + + /** + * The guard mutates bean definitions where its throw-only siblings do not, so it must run ahead + * of anything that might resolve AttributesDao. + */ + @Test + void runsAtHighestPrecedence() { + BeanFactoryPostProcessor postProcessor = guard(); + + assertTrue(postProcessor instanceof PriorityOrdered, "guard is PriorityOrdered"); + assertEquals( + Ordered.HIGHEST_PRECEDENCE, + ((PriorityOrdered) postProcessor).getOrder(), + "guard runs at highest precedence"); + } + + /** + * The first of the two defects this revision fixes. A second bean assignable to our own DAO type, + * registered under a different name, used to satisfy an "is one of ours present?" boolean: both + * survived and injection was ambiguous. + * + *

Such a bean is a peer implementation somebody registered on purpose, not ThingsBoard's. The + * guard has no standing to delete it, so it refuses and leaves the context exactly as it found + * it. + */ + @Test + void anIoTDBPeerUnderAnotherNameIsRefusedAndBothSurvive() { + DefaultListableBeanFactory beanFactory = factory(true, false); + beanFactory.registerBeanDefinition( + PEER_DAO_BEAN, new RootBeanDefinition(IoTDBTableAttributesDao.class)); + + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue( + thrown.getMessage().contains(PEER_DAO_BEAN), "names the peer: " + thrown.getMessage()); + assertTrue(beanFactory.containsBeanDefinition(PEER_DAO_BEAN), "the peer survives"); + assertTrue(beanFactory.containsBeanDefinition(IOTDB_DAO_BEAN), "and so does ours"); + assertEquals( + 2, + beanFactory.getBeanNamesForType(AttributesDao.class, true, false).length, + "nothing was withdrawn"); + } + + /** + * The second defect. A third-party AttributesDao alongside ThingsBoard's own: the removability + * check used to sit inside the mutation loop, so one bean could be withdrawn before the refusal. + * + *

Every reason to stop is now evaluated first, so the host's DAO is still present after the + * throw even though it was, on its own, perfectly removable. + */ + @Test + void aThirdPartyDaoStopsTheWithdrawalOfTheHostDaoToo() { + DefaultListableBeanFactory beanFactory = factory(true, true); + beanFactory.registerBeanDefinition( + THIRD_PARTY_DAO_BEAN, new RootBeanDefinition(THIRD_PARTY_DAO_TYPE)); + + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue( + thrown.getMessage().contains(THIRD_PARTY_DAO_BEAN), + "names the bean it will not remove: " + thrown.getMessage()); + assertTrue( + beanFactory.containsBeanDefinition(THIRD_PARTY_DAO_BEAN), "the third-party bean survives"); + assertTrue( + beanFactory.containsBeanDefinition(HOST_DAO_BEAN), + "and so does the host's, though it was removable on its own"); + } + + /** Right name, wrong type: the authorisation is for one specific class, not for a bean name. */ + @Test + void aBeanUsingTheHostNameWithAnotherTypeIsRefused() { + DefaultListableBeanFactory beanFactory = factory(true, false); + beanFactory.registerBeanDefinition(HOST_DAO_BEAN, new RootBeanDefinition(THIRD_PARTY_DAO_TYPE)); + + assertThrows(IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue(beanFactory.containsBeanDefinition(HOST_DAO_BEAN), "left untouched"); + } + + /** + * Right type, wrong name: an operator who registered ThingsBoard's class themselves, under their + * own name, made a deliberate choice. Withdrawing it is not what the documented opt-in promises. + */ + @Test + void theHostTypeUnderAnotherBeanNameIsRefused() { + DefaultListableBeanFactory beanFactory = factory(true, false); + beanFactory.registerBeanDefinition( + "customJpaAttributeDao", new RootBeanDefinition(HOST_DAO_TYPE)); + + assertThrows(IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue(beanFactory.containsBeanDefinition("customJpaAttributeDao"), "left untouched"); + } + + /** + * Something else holding our bean name. Found by direct lookup rather than by filtering the type + * scan, so it is caught even when the imposter does not implement AttributesDao at all. + */ + @Test + void aBeanHoldingOurNameWithTheWrongTypeIsRefused() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition( + IOTDB_DAO_BEAN, new RootBeanDefinition(THIRD_PARTY_DAO_TYPE)); + beanFactory.registerBeanDefinition(HOST_DAO_BEAN, new RootBeanDefinition(HOST_DAO_TYPE)); + + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, () -> guard().postProcessBeanFactory(beanFactory)); + + assertTrue( + thrown.getMessage().contains("rather than an IoTDBTableAttributesDao"), + "explains what holds the name: " + thrown.getMessage()); + assertTrue(beanFactory.containsBeanDefinition(HOST_DAO_BEAN), "nothing was withdrawn"); + } +} diff --git a/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableTimeseriesAggregationIT.java b/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableTimeseriesAggregationIT.java index 302f4fa..aad140b 100644 --- a/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableTimeseriesAggregationIT.java +++ b/iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/IoTDBTableTimeseriesAggregationIT.java @@ -792,6 +792,62 @@ void maxOverNonPositiveLongOnlyAndMixedBucketsKeepsResultTypeAgainstRealIoTDB() } } + @Test + void calendarMonthBucketsFollowTheQueryTimezoneRatherThanUtc() throws Exception { + TestScope scope = + scope( + "agg_month_tz", + "55555555-5555-5555-5555-555555555509", + "66666666-6666-6666-6666-666666666609"); + bootstrapSchema(scope.database()); + try (ITableSessionPool pool = newPool(scope.database())) { + IoTDBTableConfig config = config(8); + IoTDBTableTimeseriesWriter writer = new IoTDBTableTimeseriesWriter(pool, config); + IoTDBTableTimeseriesDao dao = new IoTDBTableTimeseriesDao(pool, writer, config); + try { + // Asia/Shanghai is UTC+8 with no DST, so every calendar MONTH boundary sits exactly eight + // hours EARLIER in epoch terms than the corresponding UTC boundary asserted above: + // Jan [1672502400000,1675180800000) 31d -> midpoint 1673841600000 + // Feb [1675180800000,1677600000000) 28d -> midpoint 1676390400000 + // Mar [1677600000000,1680278400000) 31d -> midpoint 1678939200000 + // Each midpoint is 28800000 ms below the UTC midpoint used by the test above. + // + // The middle sample is the discriminator. 1675195200000 is 2023-01-31T20:00Z, which UTC + // bucketing places in JANUARY but Shanghai bucketing places in FEBRUARY (local time + // 2023-02-01T04:00+08:00). If the DAO dropped the query timezone and fell back to UTC this + // would collapse to TWO buckets carrying 60 and 7, not three carrying 10, 50 and 7 -- so + // the test fails on the value distribution, not merely on the bucket labels. + saveAll( + dao, + scope, + List.of( + entry(1673308800000L, "n", DataType.LONG, 10L), // 2023-01-10, Jan in both zones + entry(1675195200000L, "n", DataType.LONG, 50L), // Jan in UTC, Feb in Shanghai + entry(1677974400000L, "n", DataType.LONG, 7L))); // 2023-03-05, Mar in both zones + + long startTs = 1672502400000L; // 2023-01-01T00:00+08:00 + long endTs = 1680278400000L; // 2023-04-01T00:00+08:00 + long[] shanghaiMidpoints = {1673841600000L, 1676390400000L, 1678939200000L}; + + ReadTsKvQueryResult sum = + calendarAggregate(dao, scope, "n", startTs, endTs, Aggregation.SUM, "Asia/Shanghai"); + assertNumericBuckets( + sum, + shanghaiMidpoints, + new DataType[] {DataType.LONG, DataType.LONG, DataType.LONG}, + new double[] {10.0D, 50.0D, 7.0D}, + 1677974400000L); + + ReadTsKvQueryResult count = + calendarAggregate(dao, scope, "n", startTs, endTs, Aggregation.COUNT, "Asia/Shanghai"); + assertLongBuckets(count, shanghaiMidpoints, new long[] {1L, 1L, 1L}); + } finally { + dao.destroy(); + writer.destroy(); + } + } + } + private ReadTsKvQueryResult calendarAggregate( IoTDBTableTimeseriesDao dao, TestScope scope, @@ -800,12 +856,24 @@ private ReadTsKvQueryResult calendarAggregate( long endTs, Aggregation aggregation) throws Exception { + return calendarAggregate(dao, scope, key, startTs, endTs, aggregation, "UTC"); + } + + private ReadTsKvQueryResult calendarAggregate( + IoTDBTableTimeseriesDao dao, + TestScope scope, + String key, + long startTs, + long endTs, + Aggregation aggregation, + String tzId) + throws Exception { ReadTsKvQuery query = new BaseReadTsKvQuery( key, startTs, endTs, - AggregationParams.calendar(aggregation, IntervalType.MONTH, "UTC"), + AggregationParams.calendar(aggregation, IntervalType.MONTH, tzId), 100, "ASC"); return dao.findAllAsync(scope.tenantId(), scope.entityId(), List.of(query))