From 86daf89487d889fc38c1226307ea4d079e76da1c Mon Sep 17 00:00:00 2001 From: Zihan Dai <99155080+PDGGK@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:32:37 +1000 Subject: [PATCH] Make the attributes selector usable and correct a runtime dependency's scope Deploying the module into a live ThingsBoard for the first time surfaced two defects that the existing tests could not: the module had never been run inside a running ThingsBoard, only compiled and integration-tested against its types. The two were seen on different deployment attempts, and each is attributed to its own below. The attributes selector could never take effect. This one is from the end-to-end run on thingsboard/tb-node:4.3.1.2 against apache/iotdb:2.0.8-standalone. ThingsBoard gates its timeseries DAOs on database.ts.type, so for ts and ts_latest it steps aside by itself, but it offers no equivalent for attributes: at v4.3.1.2, JpaAttributeDao is a bare @Component and so registers unconditionally. AttributesDaoConflictGuard therefore always found a competing bean and failed startup, and its own advice -- remove the bean or unset the selector -- could not be followed, because a @Component cannot be un-registered from a properties file. So on that build the documented attributes opt-in was not an option a deployment could take: setting the selector produced a startup failure, every time. I have checked 4.3.1.2 and have not surveyed the release history, so this is stated for v4.3.1.2 and no wider. The guard now withdraws ThingsBoard's bean instead of refusing to start, and the withdrawal is narrow. Candidates are sorted into three classes, and nothing is mutated until every reason to stop has been evaluated: - ours, found by looking up the bean name this configuration registers rather than by filtering the type scan, so a bean that took the name while implementing something else is caught too; - ThingsBoard's own, matched on BOTH the bean name jpaAttributeDao and the resolved type org.thingsboard.server.dao.sql.attributes.JpaAttributeDao -- this one is withdrawn, with a WARN naming it, its class and the property that caused it; - anything else implementing AttributesDao -- a third-party backend, a decorator, a subclass of our own DAO under another name -- which fails startup untouched. The third class is the point. A bean the operator registered deliberately is not this module's to delete, so the guard refuses rather than guesses, and unlike the old message "remove it or unset the selector" is now advice that can actually be followed. Matching on the class name rather than on assignability also means an upstream rename fails closed: the bean becomes unrecognised and startup stops naming it, instead of the guard withdrawing something it should not have. What the guard guarantees is bounded by what it can see. Candidates come from one getBeanNamesForType(type, true, false) snapshot, which does not initialise FactoryBeans and does not consult a parent factory, so a definition registered by a later post-processor, produced by an opaque FactoryBean, or inherited from an ancestor context is outside it. The javadoc says so rather than claiming more. Our own DAO bean also had to lose @ConditionalOnMissingBean(type = ATTRIBUTES_DAO_CLASS_NAME) -- the string form, chosen so that evaluating the condition does not load ThingsBoard classes. It is evaluated while configuration classes are parsed, strictly before the post-processor runs, so on a stock 4.3.1.2 it skipped our bean on every deployment. The second defect is in the POM. iotdb-thrift-commons was declared at test scope with a comment calling it "a transitive runtime dependency of iotdb-session" -- which is exactly what the declaration broke, since a direct declaration wins under Maven's nearest-definition rule. It carries TEndPoint, which the session pool needs at runtime, so the runtime dependency set produced from the POM omitted it and the deployed module could not create a session -- observed as a TEndPoint load failure during an earlier tb-postgres:4.2.1.1 deployment attempt, not during the 4.3.1.2 run above. That container's log was not preserved, so this is the observation rather than a quotation. It is now declared at runtime scope, which is what it is: no main source references it, the unit tests do, and the session pool needs it at runtime. That also retires the dependency:analyze suppression -- analyze-only no longer reports the artifact, so the correct scope replaces the suppression rather than sitting alongside it. One new compile-only file, src/provided/java/org/thingsboard/server/dao/sql/ attributes/JpaAttributeDao.java, joins the existing Strategy F surface. ThingsBoard's dao artifact is not on Maven Central, so without a class of that exact fully-qualified name on the test classpath the conjunctive match above could not be exercised at all. It is excluded from the packaged jar by the existing org/thingsboard/** rule and cannot shadow the real class; verified by building the jar and listing its entries. No test establishes that the name is correct -- that comes from ThingsBoard's source at v4.3.1.2, and a rename fails closed. Three in-repo documents and two production javadoc blocks change with it, because the behaviour they describe changed. IoTDBTableAttributesDao and IoTDBTableAttributesEnabledCondition each argued that no real Phase-1 deployment sets database.attributes.type -- the rationale this change overturns -- and now describe it as a selector this module supplies, pointing at the guard for what setting it does. docs/user-guide.md and docs/migration-guide.md both described a single fail-fast conflict guard for all three routes; that is still true of timeseries and latest and is no longer true of attributes, so each now describes the two behaviours separately, with the withdrawal's matching rule and its visibility boundary. README.md is included for a different reason: its attributes section argued that no shipped ThingsBoard release exposes the selector, "so a real Phase-1 deployment never sets it" -- a rationale this change overturns, since the selector is one this module supplies. Its statement that the attributes selector is independent of database.ts.type / database.ts_latest.type is retained: that is independence among this module's own routes and it remains correct. Two constants in IoTDBTableAttributesEnabledCondition widen from private to package-private so the guard's WARN can name the property and value from their single definition; the class is itself package-private, so nothing a consumer can see changes. Tests: AttributesDaoConflictGuardTest now carries 14 cases, covering the withdrawal, the zero-replacement refusal, an IoTDB peer under another name, a third-party DAO alongside a removable ThingsBoard one, right-name/wrong-type and right-type/wrong-name, a pre-built singleton with no definition to withdraw, the WARN's four fields, the no-op path and the ordering. Each of the three guarantees was mutation-checked: reverting the class-name half of the match, identifying our bean by assignability, and moving the unknown-bean check after the removal each reddened exactly one test, with an unrelated test green throughout as a contamination control. IoTDBTableTimeseriesAggregationIT gains non-UTC calendar coverage with a boundary-straddling sample -- 2023-01-31T20:00Z is January under UTC and February under Asia/Shanghai -- so an implementation that drops the query timezone changes the bucket count and fails; confirmed by forcing the zone to UTC and watching it go red. 204 unit tests and 58 integration tests pass. The full unit suite is also green against Spring 6.2.18 / Spring Boot 3.5.14, the line ThingsBoard 4.3.1.2 runs. --- iotdb-thingsboard-table/README.md | 29 +- .../docs/migration-guide.md | 35 +- iotdb-thingsboard-table/docs/user-guide.md | 56 ++- iotdb-thingsboard-table/pom.xml | 39 +- .../table/IoTDBTableAttributesDao.java | 17 +- .../IoTDBTableAttributesEnabledCondition.java | 15 +- .../table/IoTDBTableConfiguration.java | 221 ++++++++- .../dao/sql/attributes/JpaAttributeDao.java | 162 +++++++ .../table/AttributesDaoConflictGuardTest.java | 444 ++++++++++++++++++ .../IoTDBTableTimeseriesAggregationIT.java | 70 ++- 10 files changed, 1010 insertions(+), 78 deletions(-) create mode 100644 iotdb-thingsboard-table/src/provided/java/org/thingsboard/server/dao/sql/attributes/JpaAttributeDao.java create mode 100644 iotdb-thingsboard-table/src/test/java/org/apache/iotdb/extras/thingsboard/table/AttributesDaoConflictGuardTest.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..6bb0985 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 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))