Skip to content

Commit 8054a0c

Browse files
alzimmermsftalzimmermsft
authored
Added Custom Deserializers to Handle Interspersed List Responses (#16702)
* Added custom serializer to handle PageList response * Add test playback record * Add CustomPageListingDeserializer annotation in Swagger transform * Added RevApi suppression for new annotation on PageList * Fix copy and paste error * Fix Share File range listing deserialization, added more unit testing
1 parent 2d43ad8 commit 8054a0c

30 files changed

+3344
-166
lines changed

‎eng/code-quality-reports/src/main/resources/revapi/revapi.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,11 @@
241241
"old": "class com.azure.messaging.eventhubs.EventHubClientBuilder",
242242
"new": "class com.azure.messaging.eventhubs.EventHubClientBuilder",
243243
"justification": "Setting protocol to AMQP in @ServiceClientBuilder annotation is not a breaking change"
244+
},
245+
{
246+
"code": "java.annotation.added",
247+
"new": "class com.azure.storage.blob.models.PageList",
248+
"justification": "Annotation required to resolve deserialization bug."
244249
}
245250
]
246251
}

‎sdk/storage/azure-storage-blob/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Release History
22

33
## 12.9.0-beta.3 (Unreleased)
4-
4+
- Fixed a bug where interspersed element types returned by page listing would deserialize incorrectly.
55

66
## 12.9.0-beta.2 (2020-10-08)
77
- Added support to specify whether or not a pipeline policy should be added per call or per retry.

‎sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/models/PageList.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
import com.azure.core.annotation.Fluent;
88
import com.fasterxml.jackson.annotation.JsonProperty;
9+
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
910
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement;
11+
1012
import java.util.ArrayList;
1113
import java.util.List;
1214

@@ -15,6 +17,7 @@
1517
*/
1618
@JacksonXmlRootElement(localName = "PageList")
1719
@Fluent
20+
@JsonDeserialize(using = PageListDeserializer.class)
1821
public final class PageList {
1922
/*
2023
* The pageRange property.

‎sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/models/PageListDeserializer.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.azure.storage.blob.models;
5+
6+
import com.fasterxml.jackson.core.JsonParser;
7+
import com.fasterxml.jackson.core.JsonToken;
8+
import com.fasterxml.jackson.core.JsonTokenId;
9+
import com.fasterxml.jackson.databind.DeserializationContext;
10+
import com.fasterxml.jackson.databind.JsonDeserializer;
11+
12+
import java.io.IOException;
13+
import java.util.ArrayList;
14+
15+
/**
16+
* Custom Jackson JsonDeserializer that handles deserializing PageList responses.
17+
* <p>
18+
* PageList responses intersperse PageRange and ClearRange elements, without this deserializer if we received the
19+
* following response the resulting PageList would only contain one PageRange element and one ClearRange element.
20+
*
21+
* <pre>
22+
* {@code
23+
* <?xml version="1.0" encoding="utf-8"?>
24+
* <PageList>
25+
* <PageRange>
26+
* <Start>Start Byte</Start>
27+
* <End>End Byte</End>
28+
* </PageRange>
29+
* <ClearRange>
30+
* <Start>Start Byte</Start>
31+
* <End>End Byte</End>
32+
* </ClearRange>
33+
* <PageRange>
34+
* <Start>Start Byte</Start>
35+
* <End>End Byte</End>
36+
* </PageRange>
37+
* </PageList>
38+
* }
39+
* </pre>
40+
*
41+
* With the custom deserializer the response correctly returns two PageRange elements and one ClearRange element.
42+
*/
43+
final class PageListDeserializer extends JsonDeserializer<PageList> {
44+
@Override
45+
public PageList deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
46+
ArrayList<PageRange> pageRanges = new ArrayList<>();
47+
ArrayList<ClearRange> clearRanges = new ArrayList<>();
48+
49+
// Get the deserializer that handles PageRange.
50+
JsonDeserializer<Object> pageRangeDeserializer =
51+
ctxt.findRootValueDeserializer(ctxt.constructType(PageRange.class));
52+
53+
// Get the deserializer that handles ClearRange.
54+
JsonDeserializer<Object> clearRangeDeserializer =
55+
ctxt.findRootValueDeserializer(ctxt.constructType(ClearRange.class));
56+
57+
for (JsonToken currentToken = p.nextToken(); currentToken.id() != JsonTokenId.ID_END_OBJECT;
58+
currentToken = p.nextToken()) {
59+
// Get to the root element of the next item.
60+
p.nextToken();
61+
62+
if (p.getCurrentName().equals("PageRange")) {
63+
// Current token is the node that begins a PageRange object.
64+
pageRanges.add((PageRange) pageRangeDeserializer.deserialize(p, ctxt));
65+
} else if (p.getCurrentName().equals("ClearRange")) {
66+
// Current token is the node that begins a ClearRange object.
67+
clearRanges.add((ClearRange) clearRangeDeserializer.deserialize(p, ctxt));
68+
}
69+
}
70+
71+
return new PageList().setPageRange(pageRanges).setClearRange(clearRanges);
72+
}
73+
}

‎sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/HelperTest.groovy‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33

44
package com.azure.storage.blob.specialized
55

6+
import com.azure.core.util.serializer.JacksonAdapter
7+
import com.azure.core.util.serializer.SerializerEncoding
68
import com.azure.storage.blob.APISpec
79
import com.azure.storage.blob.BlobContainerAsyncClient
810
import com.azure.storage.blob.BlobUrlParts
911
import com.azure.storage.blob.implementation.util.BlobSasImplUtil
1012
import com.azure.storage.blob.models.BlobRange
13+
import com.azure.storage.blob.models.PageList
1114
import com.azure.storage.blob.sas.BlobSasPermission
1215
import com.azure.storage.blob.sas.BlobSasServiceVersion
1316
import com.azure.storage.blob.sas.BlobServiceSasSignatureValues
@@ -142,4 +145,30 @@ class HelperTest extends APISpec {
142145
.assertNext(){buffer -> assert buffer.compareTo(ByteBuffer.wrap(data)) == 0 }
143146
.verifyComplete()
144147
}
148+
149+
def "PageList custom deserializer"() {
150+
setup:
151+
def responseXml = "<?xml version=\"1.0\" encoding=\"utf-8\"?> \n" +
152+
"<PageList> \n" +
153+
" <PageRange> \n" +
154+
" <Start>0</Start> \n" +
155+
" <End>511</End> \n" +
156+
" </PageRange> \n" +
157+
" <ClearRange> \n" +
158+
" <Start>512</Start> \n" +
159+
" <End>1023</End> \n" +
160+
" </ClearRange> \n" +
161+
" <PageRange> \n" +
162+
" <Start>1024</Start> \n" +
163+
" <End>2047</End> \n" +
164+
" </PageRange> \n" +
165+
"</PageList>"
166+
167+
when:
168+
def pageList = (PageList) new JacksonAdapter().deserialize(responseXml, PageList.class, SerializerEncoding.XML)
169+
170+
then:
171+
pageList.getPageRange().size() == 2
172+
pageList.getClearRange().size() == 1
173+
}
145174
}

‎sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/PageBlobAPITest.groovy‎

Lines changed: 87 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,25 @@
44
package com.azure.storage.blob.specialized
55

66
import com.azure.core.exception.UnexpectedLengthException
7+
import com.azure.core.util.CoreUtils
78
import com.azure.storage.blob.APISpec
89
import com.azure.storage.blob.BlobContainerClient
9-
import com.azure.storage.blob.models.PageBlobCopyIncrementalRequestConditions
1010
import com.azure.storage.blob.models.BlobErrorCode
1111
import com.azure.storage.blob.models.BlobHttpHeaders
1212
import com.azure.storage.blob.models.BlobRange
1313
import com.azure.storage.blob.models.BlobRequestConditions
1414
import com.azure.storage.blob.models.BlobStorageException
15+
import com.azure.storage.blob.models.ClearRange
1516
import com.azure.storage.blob.models.CopyStatusType
16-
import com.azure.storage.blob.options.BlobGetTagsOptions
17-
import com.azure.storage.blob.options.PageBlobCopyIncrementalOptions
18-
import com.azure.storage.blob.options.PageBlobCreateOptions
17+
import com.azure.storage.blob.models.PageBlobCopyIncrementalRequestConditions
1918
import com.azure.storage.blob.models.PageBlobRequestConditions
2019
import com.azure.storage.blob.models.PageRange
2120
import com.azure.storage.blob.models.PublicAccessType
2221
import com.azure.storage.blob.models.SequenceNumberActionType
22+
import com.azure.storage.blob.options.BlobGetTagsOptions
23+
import com.azure.storage.blob.options.PageBlobCopyIncrementalOptions
24+
import com.azure.storage.blob.options.PageBlobCreateOptions
25+
import com.azure.storage.common.implementation.Constants
2326
import spock.lang.Ignore
2427
import spock.lang.Unroll
2528

@@ -196,7 +199,7 @@ class PageBlobAPITest extends APISpec {
196199
null | null | garbageEtag | null | null | null
197200
null | null | null | receivedEtag | null | null
198201
null | null | null | null | garbageLeaseID | null
199-
null | null | null | null | null | "\"notfoo\" = 'notbar'"
202+
null | null | null | null | null | "\"notfoo\" = 'notbar'"
200203
}
201204

202205
def "Create error"() {
@@ -756,7 +759,7 @@ class PageBlobAPITest extends APISpec {
756759
null | null | garbageEtag | null | null | null
757760
null | null | null | receivedEtag | null | null
758761
null | null | null | null | garbageLeaseID | null
759-
null | null | null | null | null | "\"notfoo\" = 'notbar'"
762+
null | null | null | null | null | "\"notfoo\" = 'notbar'"
760763
}
761764

762765
def "Get page ranges error"() {
@@ -770,32 +773,81 @@ class PageBlobAPITest extends APISpec {
770773
thrown(BlobStorageException)
771774
}
772775

776+
@Unroll
773777
def "Get page ranges diff"() {
774778
setup:
775-
bc.create(PageBlobClient.PAGE_BYTES * 2, true)
779+
bc.create(4 * Constants.MB, true)
776780

777-
bc.uploadPages(new PageRange().setStart(PageBlobClient.PAGE_BYTES).setEnd(PageBlobClient.PAGE_BYTES * 2 - 1),
778-
new ByteArrayInputStream(getRandomByteArray(PageBlobClient.PAGE_BYTES)))
781+
bc.uploadPages(new PageRange().setStart(0).setEnd(4 * Constants.MB - 1),
782+
new ByteArrayInputStream(getRandomByteArray(4 * Constants.MB)))
779783

780784
def snapId = bc.createSnapshot().getSnapshotId()
781785

782-
bc.uploadPages(new PageRange().setStart(0).setEnd(PageBlobClient.PAGE_BYTES - 1),
783-
new ByteArrayInputStream(getRandomByteArray(PageBlobClient.PAGE_BYTES)))
786+
rangesToUpdate.forEach({
787+
bc.uploadPages(it, new ByteArrayInputStream(getRandomByteArray((int) (it.getEnd() - it.getStart()) + 1)))
788+
})
784789

785-
bc.clearPages(new PageRange().setStart(PageBlobClient.PAGE_BYTES).setEnd(PageBlobClient.PAGE_BYTES * 2 - 1))
790+
rangesToClear.forEach({ bc.clearPages(it) })
786791

787792
when:
788-
def response = bc.getPageRangesDiffWithResponse(new BlobRange(0, PageBlobClient.PAGE_BYTES * 2), snapId, null, null, null)
793+
def response = bc.getPageRangesDiffWithResponse(new BlobRange(0, 4 * Constants.MB), snapId, null, null, null)
789794

790795
then:
791-
response.getValue().getPageRange().size() == 1
792-
response.getValue().getPageRange().get(0).getStart() == 0
793-
response.getValue().getPageRange().get(0).getEnd() == PageBlobClient.PAGE_BYTES - 1
794-
response.getValue().getClearRange().size() == 1
795-
response.getValue().getClearRange().get(0).getStart() == PageBlobClient.PAGE_BYTES
796-
response.getValue().getClearRange().get(0).getEnd() == PageBlobClient.PAGE_BYTES * 2 - 1
797796
validateBasicHeaders(response.getHeaders())
798-
Integer.parseInt(response.getHeaders().getValue("x-ms-blob-content-length")) == PageBlobClient.PAGE_BYTES * 2
797+
response.getValue().getPageRange().size() == expectedPageRanges.size()
798+
response.getValue().getClearRange().size() == expectedClearRanges.size()
799+
800+
for (def i = 0; i < expectedPageRanges.size(); i++) {
801+
def actualRange = response.getValue().getPageRange().get(i)
802+
def expectedRange = expectedPageRanges.get(i)
803+
expectedRange.getStart() == actualRange.getStart()
804+
expectedRange.getEnd() == actualRange.getEnd()
805+
}
806+
807+
for (def i = 0; i < expectedClearRanges.size(); i++) {
808+
def actualRange = response.getValue().getClearRange().get(i)
809+
def expectedRange = expectedClearRanges.get(i)
810+
expectedRange.getStart() == actualRange.getStart()
811+
expectedRange.getEnd() == actualRange.getEnd()
812+
}
813+
814+
Integer.parseInt(response.getHeaders().getValue("x-ms-blob-content-length")) == 4 * Constants.MB
815+
816+
where:
817+
rangesToUpdate | rangesToClear | expectedPageRanges | expectedClearRanges
818+
createPageRanges() | createPageRanges() | createPageRanges() | createClearRanges()
819+
createPageRanges(0, 511) | createPageRanges() | createPageRanges(0, 511) | createClearRanges()
820+
createPageRanges() | createPageRanges(0, 511) | createPageRanges() | createClearRanges(0, 511)
821+
createPageRanges(0, 511) | createPageRanges(512, 1023) | createPageRanges(0, 511) | createClearRanges(512, 1023)
822+
createPageRanges(0, 511, 1024, 1535) | createPageRanges(512, 1023, 1536, 2047) | createPageRanges(0, 511, 1024, 1535) | createClearRanges(512, 1023, 1536, 2047)
823+
}
824+
825+
static def createPageRanges(long... offsets) {
826+
def pageRanges = [] as List<PageRange>
827+
828+
if (CoreUtils.isNullOrEmpty(offsets)) {
829+
return pageRanges
830+
}
831+
832+
for (def i = 0; i < offsets.length / 2; i++) {
833+
pageRanges.add(new PageRange().setStart(offsets[i * 2]).setEnd(offsets[i * 2 + 1]))
834+
}
835+
836+
return pageRanges
837+
}
838+
839+
static def createClearRanges(long... offsets) {
840+
def clearRanges = [] as List<ClearRange>
841+
842+
if (CoreUtils.isNullOrEmpty(offsets)) {
843+
return clearRanges
844+
}
845+
846+
for (def i = 0; i < offsets.length / 2; i++) {
847+
clearRanges.add(new ClearRange().setStart(offsets[i * 2]).setEnd(offsets[i * 2 + 1]))
848+
}
849+
850+
return clearRanges
799851
}
800852

801853
def "Get page ranges diff min"() {
@@ -867,7 +919,7 @@ class PageBlobAPITest extends APISpec {
867919
null | null | garbageEtag | null | null | null
868920
null | null | null | receivedEtag | null | null
869921
null | null | null | null | garbageLeaseID | null
870-
null | null | null | null | null | "\"notfoo\" = 'notbar'"
922+
null | null | null | null | null | "\"notfoo\" = 'notbar'"
871923
}
872924

873925
def "Get page ranges diff error"() {
@@ -1178,13 +1230,13 @@ class PageBlobAPITest extends APISpec {
11781230
bu2.copyIncrementalWithResponse(new PageBlobCopyIncrementalOptions(bc.getBlobUrl(), snapshot).setRequestConditions(mac), null, null).getStatusCode() == 202
11791231

11801232
where:
1181-
modified | unmodified | match | noneMatch | tags
1182-
null | null | null | null | null
1183-
oldDate | null | null | null | null
1184-
null | newDate | null | null | null
1185-
null | null | receivedEtag | null | null
1186-
null | null | null | garbageEtag | null
1187-
null | null | null | null | "\"foo\" = 'bar'"
1233+
modified | unmodified | match | noneMatch | tags
1234+
null | null | null | null | null
1235+
oldDate | null | null | null | null
1236+
null | newDate | null | null | null
1237+
null | null | receivedEtag | null | null
1238+
null | null | null | garbageEtag | null
1239+
null | null | null | null | "\"foo\" = 'bar'"
11881240
}
11891241

11901242
@Unroll
@@ -1204,18 +1256,18 @@ class PageBlobAPITest extends APISpec {
12041256
.setTagsConditions(tags)
12051257

12061258
when:
1207-
bu2.copyIncrementalWithResponse(new PageBlobCopyIncrementalOptions(bc.getBlobUrl(), snapshot).setRequestConditions(mac),null, null)
1259+
bu2.copyIncrementalWithResponse(new PageBlobCopyIncrementalOptions(bc.getBlobUrl(), snapshot).setRequestConditions(mac), null, null)
12081260

12091261
then:
12101262
thrown(BlobStorageException)
12111263

12121264
where:
1213-
modified | unmodified | match | noneMatch | tags
1214-
newDate | null | null | null | null
1215-
null | oldDate | null | null | null
1216-
null | null | garbageEtag | null | null
1217-
null | null | null | receivedEtag | null
1218-
null | null | null | null | "\"notfoo\" = 'notbar'"
1265+
modified | unmodified | match | noneMatch | tags
1266+
newDate | null | null | null | null
1267+
null | oldDate | null | null | null
1268+
null | null | garbageEtag | null | null
1269+
null | null | null | receivedEtag | null
1270+
null | null | null | null | "\"notfoo\" = 'notbar'"
12191271
}
12201272

12211273
def "Start incremental copy error"() {

0 commit comments

Comments
 (0)