Commit ed8933d
committed
[improve][client] Introduce PulsarApiMessageId to access fields of MessageIdData
### Motivation
Currently the `MessageId` interface hiddens all fields of the
`MessageIdData` struct defined in `PulsarApi.proto`. It's usually enough
for application users because they don't need to access the fields. But
for client developers and developers of other Pulsar ecosystems (e.g.
the built-in Kafka connector and the Flink connector in another repo),
the `MessageId` interface is too simple and there is no common used
abstraction. We can see many code usages like:
```java
if (msgId instanceof BatchMessageIdImpl) {
// Do type cast and then access fields like ledger id...
} else if (msgId instanceof MessageIdImpl) {
// Do type cast and then access fields like ledger id...
// NOTE: don't put this else if before the previous one because
// BatchMessageIdImpl is also a MessageIdImpl
} // ...
```
These `MessageId` implementations are used directly. It's a very bad
design because any change to the public APIs of these implementations
could bring breaking changes.
Also, there is a `TopicMessageIdImpl` that each time a
`getInnerMessageId()` method must be used to get the underlying
`MessageId` object, then do the type assertion and cast again. It makes
code unnecessarily complicated.
### Modifications
Introduce the `PulsarApiMessageId` interface into the `pulsar-common`
module. All `MessageId` implementations so far (except `MultiMessageId`)
should extend this interface so we can do the following conversion
safely in client code or other modules:
```java
long ledgerId = ((PulsarApiMessageId) msgId).getLedgerId();
```
Regarding the `ack_set` field, use a `BitSet` instead of the
`BatchMessageAcker` to record if a message in the batch is acknowledged.
Since the `TopicMessageId` is just a proxy of other `MessageId`
implementations, it's stored as key or value in the map directly because
the `compareTo`/`equal`/`hashCode` methods have the same semantics with
the underlying `MessageId`. There is no need to cast the type and call
`getInnerMessageId`.
Remove all other usages and mark the public methods as deprecated to
avoid breaking changes. They could be removed in the next major release.1 parent 9917aac commit ed8933d
File tree
34 files changed
+563
-469
lines changed- pulsar-broker/src/test/java/org/apache/pulsar
- broker/service
- client
- api
- impl
- pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli
- pulsar-client/src
- main/java/org/apache/pulsar/client
- impl
- util
- test/java/org/apache/pulsar/client/impl
- pulsar-common/src/main/java/org/apache/pulsar/client/api
- pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils
- pulsar-io/kafka-connect-adaptor/src/main/java/org/apache/pulsar/io/kafka/connect
34 files changed
+563
-469
lines changedLines changed: 3 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
48 | | - | |
| 47 | + | |
49 | 48 | | |
50 | 49 | | |
51 | 50 | | |
| |||
337 | 336 | | |
338 | 337 | | |
339 | 338 | | |
340 | | - | |
341 | | - | |
| 339 | + | |
342 | 340 | | |
343 | 341 | | |
344 | 342 | | |
| |||
354 | 352 | | |
355 | 353 | | |
356 | 354 | | |
357 | | - | |
358 | | - | |
| 355 | + | |
359 | 356 | | |
360 | 357 | | |
361 | 358 | | |
| |||
Lines changed: 1 addition & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
54 | 53 | | |
55 | 54 | | |
56 | 55 | | |
| |||
679 | 678 | | |
680 | 679 | | |
681 | 680 | | |
682 | | - | |
683 | | - | |
| 681 | + | |
684 | 682 | | |
685 | 683 | | |
686 | 684 | | |
| |||
Lines changed: 1 addition & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
43 | 42 | | |
44 | 43 | | |
45 | 44 | | |
| |||
768 | 767 | | |
769 | 768 | | |
770 | 769 | | |
771 | | - | |
| 770 | + | |
772 | 771 | | |
773 | 772 | | |
774 | 773 | | |
| |||
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
| 42 | + | |
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
| |||
176 | 177 | | |
177 | 178 | | |
178 | 179 | | |
179 | | - | |
| 180 | + | |
180 | 181 | | |
181 | 182 | | |
182 | | - | |
| 183 | + | |
183 | 184 | | |
184 | 185 | | |
185 | 186 | | |
| |||
Lines changed: 0 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
121 | | - | |
122 | | - | |
123 | | - | |
124 | 121 | | |
125 | 122 | | |
126 | 123 | | |
| |||
166 | 163 | | |
167 | 164 | | |
168 | 165 | | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | 166 | | |
173 | 167 | | |
174 | 168 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | | - | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
291 | 291 | | |
292 | 292 | | |
293 | 293 | | |
294 | | - | |
| 294 | + | |
295 | 295 | | |
296 | 296 | | |
297 | 297 | | |
| |||
Lines changed: 3 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| 41 | + | |
41 | 42 | | |
42 | | - | |
43 | | - | |
44 | 43 | | |
45 | 44 | | |
46 | 45 | | |
| |||
611 | 610 | | |
612 | 611 | | |
613 | 612 | | |
614 | | - | |
615 | | - | |
| 613 | + | |
| 614 | + | |
616 | 615 | | |
617 | 616 | | |
618 | 617 | | |
619 | | - | |
620 | 618 | | |
621 | 619 | | |
622 | 620 | | |
| |||
Lines changed: 6 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| 58 | + | |
58 | 59 | | |
59 | 60 | | |
60 | 61 | | |
| |||
1192 | 1193 | | |
1193 | 1194 | | |
1194 | 1195 | | |
| 1196 | + | |
1195 | 1197 | | |
1196 | | - | |
1197 | 1198 | | |
1198 | 1199 | | |
1199 | 1200 | | |
1200 | | - | |
1201 | 1201 | | |
1202 | 1202 | | |
1203 | 1203 | | |
| |||
1251 | 1251 | | |
1252 | 1252 | | |
1253 | 1253 | | |
1254 | | - | |
1255 | | - | |
| 1254 | + | |
| 1255 | + | |
1256 | 1256 | | |
1257 | 1257 | | |
1258 | 1258 | | |
1259 | | - | |
1260 | 1259 | | |
1261 | 1260 | | |
1262 | 1261 | | |
| |||
1310 | 1309 | | |
1311 | 1310 | | |
1312 | 1311 | | |
1313 | | - | |
1314 | | - | |
| 1312 | + | |
| 1313 | + | |
1315 | 1314 | | |
1316 | 1315 | | |
1317 | 1316 | | |
1318 | | - | |
1319 | 1317 | | |
1320 | 1318 | | |
1321 | 1319 | | |
| |||
Lines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
34 | | - | |
| 35 | + | |
| 36 | + | |
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| |||
0 commit comments