tmaret commented on a change in pull request #101:
URL:
https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/101#discussion_r821460991
##########
File path:
src/main/java/org/apache/sling/distribution/journal/bookkeeper/PackageEvent.java
##########
@@ -33,22 +33,23 @@
import org.osgi.service.event.Event;
@ParametersAreNonnullByDefault
-class ImportedEvent {
+class PackageEvent {
Review comment:
Suggest to `s/PackageEvent/AppliedEvent`
##########
File path:
src/main/java/org/apache/sling/distribution/journal/bookkeeper/PackageEvent.java
##########
@@ -33,22 +33,23 @@
import org.osgi.service.event.Event;
@ParametersAreNonnullByDefault
-class ImportedEvent {
+class PackageEvent {
public static final String PACKAGE_ID = "distribution.package.id";
- private static final String KIND_IMPORTER = "importer";
Review comment:
Please keep this constant instead of passing the component kind through
the constructor. Although `KIND_IMPORTER` and `SUBSERVICE_BOOKKEEPER` have the
same value, they mean two different things.
##########
File path:
src/main/java/org/apache/sling/distribution/journal/impl/subscriber/DistributionSubscriber.java
##########
@@ -359,14 +361,17 @@ private void blockingSendStoredStatus() throws
InterruptedException, IOException
throw new InterruptedException("Shutting down");
}
- private void processQueueItem(FullMessage<PackageMessage> item) throws
PersistenceException, LoginException, DistributionException {
+ private void processQueueItem(FullMessage<PackageMessage> item) throws
PersistenceException, LoginException, DistributionException,
ImportPostProcessException {
MessageInfo info = item.getInfo();
PackageMessage pkgMsg = item.getMessage();
boolean skip = shouldSkip(info.getOffset());
+ PackageMessage.ReqType type = pkgMsg.getReqType();
try {
idleCheck.busy(bookKeeper.getRetries(pkgMsg.getPubAgentName()));
if (skip) {
bookKeeper.removePackage(pkgMsg, info.getOffset());
+ } else if (type.equals(INVALIDATE)) {
Review comment:
Please use the equality operator `==` with enums.
```
type == INVALIDATE
```
##########
File path:
src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeper.java
##########
@@ -165,14 +165,33 @@ public void importPackage(PackageMessage pkgMsg, long
offset, long createdTime)
packageRetries.clear(pkgMsg.getPubAgentName());
- Event event = new ImportedEvent(pkgMsg,
config.getSubAgentName()).toEvent();
+ Event event = new PackageEvent(pkgMsg, config.getSubAgentName(),
SUBSERVICE_IMPORTER).toEvent();
eventAdmin.postEvent(event);
log.info("Imported distribution package {} at offset={}", pkgMsg,
offset);
} catch (DistributionException | LoginException | IOException |
RuntimeException | ImportPostProcessException e) {
failure(pkgMsg, offset, e);
}
}
-
+
+ public void invalidateCache(PackageMessage pkgMsg, long offset) throws
LoginException, PersistenceException, ImportPostProcessException {
Review comment:
Please catch exceptions like with the `importPackage` and invoke the
`failure` method which handles the retries policy.
##########
File path:
src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeper.java
##########
@@ -165,14 +165,33 @@ public void importPackage(PackageMessage pkgMsg, long
offset, long createdTime)
packageRetries.clear(pkgMsg.getPubAgentName());
- Event event = new ImportedEvent(pkgMsg,
config.getSubAgentName()).toEvent();
+ Event event = new PackageEvent(pkgMsg, config.getSubAgentName(),
SUBSERVICE_IMPORTER).toEvent();
eventAdmin.postEvent(event);
log.info("Imported distribution package {} at offset={}", pkgMsg,
offset);
} catch (DistributionException | LoginException | IOException |
RuntimeException | ImportPostProcessException e) {
failure(pkgMsg, offset, e);
}
}
-
+
+ public void invalidateCache(PackageMessage pkgMsg, long offset) throws
LoginException, PersistenceException, ImportPostProcessException {
Review comment:
Currently, invalidation is implicitly handled by the post processor. I
think we'd need to make it explicit. Indeed, post processors may be inactive or
have a different semantic.
I suggest to
1. add a new interface `org.apache.sling.distribution.InvalidationProcessor`
which method signature is similar to the `InvalidationProcessor` one.
2. Invoke `InvalidationProcessor` before committing the resolver and storing
the status
3. Don't invoke the post processor which is tied to package imports
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
