Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update vastModifier to group inserted impression tags with existing #1309

Merged
merged 2 commits into from
Jun 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions src/main/java/org/prebid/server/vast/VastModifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
public class VastModifier {

private static final String IN_LINE_TAG = "<InLine>";
private static final String IN_LINE_CLOSE_TAG = "</InLine>";
private static final String WRAPPER_TAG = "<Wrapper>";
private static final String WRAPPER_CLOSE_TAG = "</Wrapper>";
private static final String IMPRESSION_CLOSE_TAG = "</Impression>";
private final BidderCatalog bidderCatalog;
private final EventsService eventsService;
private final Metrics metrics;
Expand Down Expand Up @@ -88,7 +88,7 @@ public String createBidVastXml(String bidder,
}

private static String resolveVastXmlFrom(String bidAdm, String bidNurl) {
return bidAdm == null && bidNurl != null
return StringUtils.isEmpty(bidAdm) && bidNurl != null
? "<VAST version=\"3.0\"><Ad><Wrapper>"
+ "<AdSystem>prebid.org wrapper</AdSystem>"
+ "<VASTAdTagURI><![CDATA[" + bidNurl + "]]></VASTAdTagURI>"
Expand All @@ -104,28 +104,45 @@ private String appendTrackingUrlToVastXml(String vastXml, String vastUrlTracking
if (inLineTagIndex != -1) {
return appendTrackingUrlForInlineType(vastXml, vastUrlTracking);
} else if (wrapperTagIndex != -1) {
return appendTrackingUrl(vastXml, vastUrlTracking, false);
return appendTrackingUrl(vastXml, vastUrlTracking);
}
throw new PreBidException(
String.format("VastXml does not contain neither InLine nor Wrapper for %s response", bidder));
}

private String appendTrackingUrlForInlineType(String vastXml, String vastUrlTracking) {
final String closeTag = "</Impression>";
final int closeTagIndex = vastXml.indexOf(closeTag);
private static String appendTrackingUrlForInlineType(String vastXml, String vastUrlTracking) {
final int closeTagIndex = vastXml.indexOf(IMPRESSION_CLOSE_TAG);

// no impression tag - pass it as it is
if (closeTagIndex == -1) {
return vastXml;
}

return appendTrackingUrl(vastXml, vastUrlTracking, true);
return appendTrackingUrl(vastXml, vastUrlTracking);
}

private String appendTrackingUrl(String vastXml, String vastUrlTracking, boolean inline) {
final String closeTag = inline ? IN_LINE_CLOSE_TAG : WRAPPER_CLOSE_TAG;
final int indexOfCloseTag = StringUtils.indexOfIgnoreCase(vastXml, closeTag);
final String caseSpecificCloseTag = vastXml.substring(indexOfCloseTag, indexOfCloseTag + closeTag.length());
private static String appendTrackingUrl(String vastXml, String vastUrlTracking) {
if (vastXml.contains(IMPRESSION_CLOSE_TAG)) {
return insertAfterExistingImpressionTag(vastXml, vastUrlTracking);
}
return insertBeforeEndOfWrapperElement(vastXml, vastUrlTracking);
}

private static String insertAfterExistingImpressionTag(String vastXml, String vastUrlTracking) {
final String impressionTag = "<Impression><![CDATA[" + vastUrlTracking + "]]></Impression>";
final int replacementStart = vastXml.lastIndexOf(IMPRESSION_CLOSE_TAG);

return new StringBuilder().append(vastXml, 0, replacementStart)
.append(IMPRESSION_CLOSE_TAG)
.append(impressionTag)
.append(vastXml.substring(replacementStart + IMPRESSION_CLOSE_TAG.length()))
.toString();
}

private static String insertBeforeEndOfWrapperElement(String vastXml, String vastUrlTracking) {
final int indexOfCloseTag = StringUtils.indexOfIgnoreCase(vastXml, WRAPPER_CLOSE_TAG);
final String caseSpecificCloseTag =
vastXml.substring(indexOfCloseTag, indexOfCloseTag + WRAPPER_CLOSE_TAG.length());
final String impressionTag = "<Impression><![CDATA[" + vastUrlTracking + "]]></Impression>";

return vastXml.replace(caseSpecificCloseTag, impressionTag + caseSpecificCloseTag);
Expand Down
25 changes: 19 additions & 6 deletions src/test/java/org/prebid/server/vast/VastModifierTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,9 @@ public void modifyVastXmlShouldModifyVastAndAppendUrl() {

// then
final String modifiedVast = "<VAST version=\"3.0\"><Ad><Wrapper><AdSystem>prebid.org wrapper</AdSystem>"
+ "<VASTAdTagURI><![CDATA[adm2]]></VASTAdTagURI><Impression></Impression><Creatives></Creatives>"
+ "<Impression><![CDATA[http://external-url/event]]></Impression></Wrapper></Ad></VAST>";
+ "<VASTAdTagURI><![CDATA[adm2]]></VASTAdTagURI><Impression></Impression>"
+ "<Impression><![CDATA[http://external-url/event]]></Impression>"
+ "<Creatives></Creatives></Wrapper></Ad></VAST>";

assertThat(result).isEqualTo(new TextNode(modifiedVast));
}
Expand All @@ -129,7 +130,7 @@ public void createBidVastXmlShouldNotModifyWhenBidderNotAllowed() {
}

@Test
public void createBidVastXmlShouldInjectBidNurlWhenBidAdmIsEmptyAndEventsDisabledByAccount() {
public void createBidVastXmlShouldInjectBidNurlWhenBidAdmIsNullAndEventsDisabledByAccount() {
// when
final String result = target.createBidVastXml(BIDDER, null, BID_NURL, BID_ID, ACCOUNT_ID,
givenEventsContext(false), emptyList());
Expand All @@ -138,6 +139,16 @@ public void createBidVastXmlShouldInjectBidNurlWhenBidAdmIsEmptyAndEventsDisable
assertThat(result).isEqualTo(modifiedAdm(BID_NURL));
}

@Test
public void createBidVastXmlShouldInjectBidNurlWhenBidAdmIsEmptyAndEventsDisabledByAccount() {
// when
final String result = target.createBidVastXml(BIDDER, "", BID_NURL, BID_ID, ACCOUNT_ID,
givenEventsContext(false), emptyList());

// then
assertThat(result).isEqualTo(modifiedAdm(BID_NURL));
}

@Test
public void createBidVastXmlShouldReturnAdmWhenBidAdmIsPresentAndEventsDisabledByAccount() {
// when
Expand All @@ -163,17 +174,19 @@ public void createBidVastXmlShouldBeModifiedWithNewImpressionVastUrlWhenEventsEn
}

@Test
public void createBidVastXmlShouldBeModifiedWithNewImpressionAsALastChildForInLineType() {
public void createBidVastXmlShouldBeModifiedWithNewImpressionAfterExistingImpressionTags() {
// when
final String bidAdm = "<InLine><Impression>http:/test.com</Impression></InLine>";
final String bidAdm = "<InLine><Impression>http:/test.com</Impression>"
+ "<Impression>http:/test2.com</Impression><Creatives></Creatives></InLine>";
final String result = target
.createBidVastXml(BIDDER, bidAdm, BID_NURL, BID_ID, ACCOUNT_ID, eventsContext(), emptyList());

// then
verify(eventsService).vastUrlTracking(BID_ID, BIDDER, ACCOUNT_ID, AUCTION_TIMESTAMP, INTEGRATION);

assertThat(result).isEqualTo("<InLine><Impression>http:/test.com</Impression>"
+ "<Impression><![CDATA[" + VAST_URL_TRACKING + "]]></Impression></InLine>");
+ "<Impression>http:/test2.com</Impression>"
+ "<Impression><![CDATA[" + VAST_URL_TRACKING + "]]></Impression><Creatives></Creatives></InLine>");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"puts": [
{
"type": "xml",
"value": "<VAST version=\"3.0\"><Ad><Wrapper><AdSystem>prebid.org wrapper</AdSystem><VASTAdTagURI><![CDATA[adm2]]></VASTAdTagURI><Impression></Impression><Creatives></Creatives><Impression><![CDATA[http://localhost:8080/event?t=imp&b=bidid&a=14062&ts=1000&bidder=rubicon&f=b&int=]]></Impression></Wrapper></Ad></VAST>",
"value": "<VAST version=\"3.0\"><Ad><Wrapper><AdSystem>prebid.org wrapper</AdSystem><VASTAdTagURI><![CDATA[adm2]]></VASTAdTagURI><Impression></Impression><Impression><![CDATA[http://localhost:8080/event?t=imp&b=bidid&a=14062&ts=1000&bidder=rubicon&f=b&int=]]></Impression><Creatives></Creatives></Wrapper></Ad></VAST>",
"ttlseconds": 3600
}
]
Expand Down