Skip to content

Commit

Permalink
MH-13167, Republishing metadata does not update all metadata
Browse files Browse the repository at this point in the history
The merge option of the `publish-engage` operation is used for
republishing metadata like dublincore catalogs or access control lists.

Merge works in a way that every previously published element which is
not in the new catalog is included in the merged media package. Or in
other words, only newly published catalogs are updated.

This behavior, while technically well defined and implemented correctly,
causes severe usability issues in some edge cases since a metadata
update can include an unexpected removal of an element.

Example 1 (Series Catalog)
--------------------------

Given an episode which is assigned to a series. Both dublincore/episode
and dublincore/series are published to engage. The user updates the
metadata in the admin interface, removes the series and republishes the
metadata.

Since the episode does not contain a series catalog any longer, the
element is not published which causes the merge to republish the old one
(a new catalog is not published after all). Hence you end up with a
published episode without a series which still has a series catalog.

Example 2 (ACL)
---------------

Given an episode with a published episode ACL. A user now removes the
episode ACL, modifies the series ACL and republishes the metadata to
update the access status. Note that switching from episode to series ACL
may happen automatically in some cases.

The publication of the episode ACL is not updated and the merge option
will republish the old access rules which override the less specific
series ACL. Hence the new access rules have no effect at all.

The Fix
-------

This patch introduces a new merge-force-flavor option which can be used
to enforce the update of certain catalogs. By default this is done for
the dublincore catalogs and the ACLs which are usually part of a
metadata republication. This ensures the metadata are updated even when
a catalog is removed.

The default is set in a way that users will usually not need to specify
any other value. Additional values may be set for extended metadata
catalogs (for which the default does not make things worse though) or in
the case that someone republishes parts of the metadata which is rather
unlikely.
  • Loading branch information
lkiesow committed Oct 16, 2018
1 parent 1e85685 commit a121d60
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Parameter Table

|configuration keys |description |
|---------------------------|------------------------------------------------------------------------------|
|check-availability |Check if the media if rechable |
|check-availability |Check if the media if reachable |
|download-source-flavors |Specifies which media should be published for download |
|download-source-tags |Specifies which media should be published for download |
|download-target-subflavors |Subflavor to use for distributed material |
Expand All @@ -24,6 +24,8 @@ Parameter Table
|streaming-source-tags |Specifies which media should be published to the streaming server |
|streaming-target-tags |Modify tags of published media |
|streaming-target-subflavors|Subflavor to use for distributed material |
|merge-force-flavors |Flavors of elements for which an update is enforced when mergeing catalogs. |
| |Defaults to `dublincore/*,security/*`.

Operation Example
-----------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import java.net.URI;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -84,7 +85,7 @@
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.UUID;

import java.util.stream.Collectors;

/**
* The workflow definition for handling "engage publication" operations
Expand All @@ -109,6 +110,9 @@ public class PublishEngageWorkflowOperationHandler extends AbstractWorkflowOpera
private static final String STREAMING_TARGET_SUBFLAVOR = "streaming-target-subflavor";
private static final String CHECK_AVAILABILITY = "check-availability";
private static final String STRATEGY = "strategy";
private static final String MERGE_FORCE_FLAVORS = "merge-force-flavors";

private static final String MERGE_FORCE_FLAVORS_DEFAULT = "dublincore/*,security/*";

/** The default path to the player **/
protected static final String DEFAULT_PLAYER_PATH = "/engage/ui/watch.html";
Expand Down Expand Up @@ -233,6 +237,8 @@ public WorkflowOperationResult start(final WorkflowInstance workflowInstance, Jo
String streamingSourceFlavors = StringUtils.trimToEmpty(op.getConfiguration(STREAMING_SOURCE_FLAVORS));
String streamingTargetSubflavor = StringUtils.trimToNull(op.getConfiguration(STREAMING_TARGET_SUBFLAVOR));
String republishStrategy = StringUtils.trimToEmpty(op.getConfiguration(STRATEGY));
String mergeForceFlavorsStr = StringUtils.trimToEmpty(
StringUtils.defaultString(op.getConfiguration(MERGE_FORCE_FLAVORS), MERGE_FORCE_FLAVORS_DEFAULT));

boolean checkAvailability = option(op.getConfiguration(CHECK_AVAILABILITY)).bind(trimToNone).map(toBool)
.getOrElse(true);
Expand All @@ -250,6 +256,10 @@ public WorkflowOperationResult start(final WorkflowInstance workflowInstance, Jo
return createResult(mediaPackage, Action.CONTINUE);
}

// Parse forced flavors
List<MediaPackageElementFlavor> mergeForceFlavors = Arrays.stream(StringUtils.split(mergeForceFlavorsStr, ", "))
.map(MediaPackageElementFlavor::parseFlavor).collect(Collectors.toList());

// Parse the download target flavor
MediaPackageElementFlavor downloadSubflavor = null;
if (downloadTargetSubflavor != null) {
Expand Down Expand Up @@ -368,7 +378,7 @@ public WorkflowOperationResult start(final WorkflowInstance workflowInstance, Jo
switch (republishStrategy) {
case ("merge"):
// merge() returns merged mediapackage or null mediaPackage is not published
mediaPackageForSearch = merge(mediaPackageForSearch);
mediaPackageForSearch = merge(mediaPackageForSearch, mergeForceFlavors);
if (mediaPackageForSearch == null) {
logger.info("Skipping republish for {} since it is not currently published", mediaPackage.getIdentifier().toString());
return createResult(mediaPackage, Action.SKIP);
Expand Down Expand Up @@ -670,12 +680,11 @@ protected MediaPackage getDistributedMediapackage(String mediaPackageID) throws
* @return merged mediapackage or null if a published medipackage was not found
* @throws WorkflowOperationException
*/
protected MediaPackage merge(MediaPackage mediaPackageForSearch) throws WorkflowOperationException {
MediaPackage mergedMediaPackage = null;
mergedMediaPackage = mergePackages(mediaPackageForSearch,
getDistributedMediapackage(mediaPackageForSearch.toString()));

return mergedMediaPackage;
protected MediaPackage merge(MediaPackage mediaPackageForSearch, List<MediaPackageElementFlavor> forceFlavors)
throws WorkflowOperationException {
return mergePackages(mediaPackageForSearch,
getDistributedMediapackage(mediaPackageForSearch.toString()),
forceFlavors);
}

/**
Expand All @@ -693,14 +702,20 @@ protected MediaPackage merge(MediaPackage mediaPackageForSearch) throws Workflow
* the mediapackage that is currently published
* @return the merged mediapackage
*/
protected MediaPackage mergePackages(MediaPackage updatedMp, MediaPackage publishedMp) {
protected MediaPackage mergePackages(MediaPackage updatedMp, MediaPackage publishedMp,
List<MediaPackageElementFlavor> forceFlavors) {
if (publishedMp == null)
return updatedMp;

MediaPackage mergedMediaPackage = (MediaPackage) updatedMp.clone();
for (MediaPackageElement element : publishedMp.elements()) {
String type = element.getElementType().toString().toLowerCase();
if (updatedMp.getElementsByFlavor(element.getFlavor()).length == 0) {
if (forceFlavors.stream().anyMatch((f) -> element.getFlavor().matches(f))) {
logger.info("Forcing removal of {} {} due to the absence of a new element with flavor {}",
type, element.getIdentifier(), element.getFlavor().toString());
continue;
}
logger.info("Merging {} '{}' into the updated mediapackage", type, element.getIdentifier());
mergedMediaPackage.add((MediaPackageElement) element.clone());
} else {
Expand Down

0 comments on commit a121d60

Please sign in to comment.