Skip to content
This repository was archived by the owner on Jun 18, 2024. It is now read-only.

Fix for internalGetAttachments, ExtendedProperty, MapiTypeConverterMapEntry #80

Closed
avromf wants to merge 3 commits into from
Closed

Fix for internalGetAttachments, ExtendedProperty, MapiTypeConverterMapEntry #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3031da5
Fix for internalGetAttachments
avromf Sep 24, 2014
8aee4f2
ExtendedProperty incorrect for loop check
avromf Sep 24, 2014
24adcb4
MapiTypeConverterMapEntry validateValueAsArray is completely wrong
avromf Sep 24, 2014
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
12 changes: 7 additions & 5 deletions src/main/java/microsoft/exchange/webservices/data/ExchangeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
import java.util.List;
import java.util.Locale;
import java.util.TimeZone;
import java.util.concurrent.FutureTask;

import org.w3c.dom.Document;
import org.w3c.dom.Node;

Expand Down Expand Up @@ -1552,10 +1550,13 @@ private ServiceResponseCollection<GetAttachmentResponse> internalGetAttachments(
}
request.setBodyType(bodyType);

if (additionalProperties != null)
{
List propsArray = new ArrayList();
Copy link
Contributor

@vboctor vboctor Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indentation.

propsArray.add(additionalProperties);

if (additionalProperties != null) {
for (PropertyDefinitionBase propertyDefinitionBase : additionalProperties)
Copy link
Contributor

@vboctor vboctor Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use addAll() and Arrays.asList() instead of manually looping through and populating each element.

Copy link

@Noblebrown Noblebrown Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those methods cannot be used with Iterable<> as far as I know. The loop is fine, but add the properties from additionalProperties directly to the request instead of using an intermediary list which winds up populating the request anyway.

{
propsArray.add(propertyDefinitionBase);
}
request.getAdditionalProperties().addAll(propsArray);
}

Expand Down Expand Up @@ -4460,3 +4461,4 @@ public boolean autodiscoverRedirectionUrlValidationCallback(
}

}

5 changes: 2 additions & 3 deletions src/main/java/microsoft/exchange/webservices/data/ExtendedProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
package microsoft.exchange.webservices.data;

import java.util.ArrayList;

import javax.xml.stream.XMLStreamException;

/**
Expand Down Expand Up @@ -105,7 +104,7 @@ protected void writeElementsToXml(EwsServiceXmlWriter writer)
writer
.writeStartElement(XmlNamespace.Types,
XmlElementNames.Values);
for (int index = 0; index <= array.size(); index++) {
for (int index = 0; index < array.size(); index++) {
writer.writeElementValue(XmlNamespace.Types,
XmlElementNames.Value, MapiTypeConverter
.convertToString(this.getPropertyDefinition()
Expand Down Expand Up @@ -170,7 +169,7 @@ private String getStringValue() {
} else {
StringBuilder sb = new StringBuilder();
sb.append("[");
for (int index = 0; index <= array.size(); index++) {
for (int index = 0; index < array.size(); index++) {
sb.append(MapiTypeConverter.convertToString(this
.getPropertyDefinition().getMapiType(), array
.get(index)));
Expand Down
44 changes: 21 additions & 23 deletions src/main/java/microsoft/exchange/webservices/data/MapiTypeConverterMapEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
**************************************************************************/
package microsoft.exchange.webservices.data;

import java.lang.reflect.Array;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -190,29 +190,27 @@ protected Object ConvertToValueOrDefault(String stringValue) throws ServiceXmlDe
* @throws microsoft.exchange.webservices.data.ArgumentException
* the argument exception
*/
private void validateValueAsArray(Object value) throws ArgumentException {

Array array = null;
if(value instanceof Array)
{
array = (Array) value;
}

if (array == null) {
throw new ArgumentException(String.format(
Strings.IncompatibleTypeForArray, value.getClass(), this
.getType()));
private void validateValueAsArray(Object value) throws ArgumentException
Copy link
Contributor

@vboctor vboctor Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good candidate for unit tests.

{
ArrayList array = null;
Copy link
Contributor

@vboctor vboctor Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First thing is to handle value itself being null. Otherwise first reference of value later will throw.

if (value instanceof ArrayList)
{
array = (ArrayList) value;
}
if (array == null)
{
throw new ArgumentException(String.format(Strings.IncompatibleTypeForArray, value.getClass(), this.getType()));
}
else if (array.isEmpty())
{
throw new ArgumentException(Strings.ArrayMustHaveAtLeastOneElement);
}
else if (array.get(0).getClass() != this.getType())
Copy link
Contributor

@vboctor vboctor Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check the type for all the elements?

{
throw new ArgumentException(String.format(Strings.IncompatibleTypeForArray, value.getClass(), this.getType()));
}
}

} else if (getDim(array) != 1) {
throw new ArgumentException(Strings.ArrayMustHaveSingleDimension);
} else if (Array.getLength(array) == 0) {
throw new ArgumentException(Strings.ArrayMustHaveAtLeastOneElement);
} else if (array.getClass().getComponentType() != this.getType()) {
throw new ArgumentException(String.format(
Strings.IncompatibleTypeForArray, value.getClass(), this
.getType()));
}
}

/**
* Gets the dim. If `array' is an array object returns its dimensions;
Expand Down