diff --git a/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/DomainObjectFactoryImpl.java b/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/DomainObjectFactoryImpl.java index 78fbb75d3b2..a5d5656e7d1 100644 --- a/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/DomainObjectFactoryImpl.java +++ b/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/DomainObjectFactoryImpl.java @@ -337,10 +337,12 @@ public class DomainObjectFactoryImpl implements DomainObjectFactory { } public static TimePoint loadTimePoint(Document object, String fieldName) { - TimePoint result = null; + final TimePoint result; Number timePointAsNumber = (Number) object.get(fieldName); if (timePointAsNumber != null) { result = new MillisecondsTimePoint(timePointAsNumber.longValue()); + } else { + result = null; } return result; } @@ -351,12 +353,15 @@ public class DomainObjectFactoryImpl implements DomainObjectFactory { public static TimeRange loadTimeRange(Document object, FieldNames field) { Document timeRangeObj = (Document) object.get(field.name()); + final TimeRange result; if (timeRangeObj == null) { - return null; + result = null; + } else { + TimePoint from = loadTimePoint(timeRangeObj, FieldNames.FROM_MILLIS); + TimePoint to = loadTimePoint(timeRangeObj, FieldNames.TO_MILLIS); + result = new TimeRangeImpl(from, to); } - TimePoint from = loadTimePoint(timeRangeObj, FieldNames.FROM_MILLIS); - TimePoint to = loadTimePoint(timeRangeObj, FieldNames.TO_MILLIS); - return new TimeRangeImpl(from, to); + return result; } /** @@ -1165,7 +1170,7 @@ public class DomainObjectFactoryImpl implements DomainObjectFactory { UUID id = (UUID) eventDBObject.get(FieldNames.EVENT_ID.name()); TimePoint startDate = loadTimePoint(eventDBObject, FieldNames.EVENT_START_DATE); TimePoint endDate = loadTimePoint(eventDBObject, FieldNames.EVENT_END_DATE); - if (endDate.before(startDate)) { + if (endDate != null && startDate != null && endDate.before(startDate)) { logger.warning("End date "+endDate+" of event "+name+" with ID "+id+" is before its start date "+ startDate+"; adjusting such that end date equals start date."); endDate = startDate; diff --git a/java/com.sap.sailing.domain.test/resources/orc/03600000HUH.json b/java/com.sap.sailing.domain.test/resources/orc/03600000HUH.json new file mode 100644 index 00000000000..38a3e920af9 --- /dev/null +++ b/java/com.sap.sailing.domain.test/resources/orc/03600000HUH.json @@ -0,0 +1,3 @@ +{"rms":[ +{"NatAuth":"UKR","BIN":"UKR0267_Karina","CertNo":"0267_4","RefNo":"03600000HUH","SailNo":"UKR 267","YachtName":"Karina","Class":"Teliga 89","Builder":"im. L Teligi","Designer":"L. Teliga","Address3":"YC Obolon","C_Type":"CLUB","Family":"ORC","Division":"C","IssueDate":"2020-05-07T11:48:07.143Z","Age_Year":1975,"CrewWT":460,"LOA":8.95,"IMSL":7.354,"Draft":1.562,"MB":3.092,"Dspl_Measurement":3790,"Stability_Index":107.3,"Dynamic_Allowance":0.328,"GPH":725.4,"TMF_Inshore":0.8568,"ILCWA":787.8,"TMF_Offshore":0.849,"OSN":706.7,"CDL":6.694,"TN_Offshore_Low":0.8193,"TN_Offshore_Medium":1.0478,"TN_Offshore_High":1.1557,"TN_Inshore_Low":0.6372,"TN_Inshore_Medium":0.8543,"TN_Inshore_High":0.9652,"TND_Offshore_Low":823.9,"TND_Offshore_Medium":644.2,"TND_Offshore_High":584.1,"TND_Inshore_Low":1059.3,"TND_Inshore_Medium":790.1,"TND_Inshore_High":699.3,"OSN_Jibs":735.4,"TMF_Jibs":0.8158,"Pred_Up_TOD":708.3,"Pred_Down_TOD":650.3,"Pred_Up_TOT":0.8471,"Pred_Down_TOT":0.9227,"Dspl_Sailing":4377,"WSS":18.58,"Area_Main":17.88,"Area_Jib":33.38,"Area_Sym":71.46,"Area_ASym":56.06,"GPH_Jibs":757.5,"GPH_Jibs_TOT":0.7921,"Allowances":{"WindSpeeds":[6,8,10,12,14,16,20],"WindAngles":[52,60,75,90,110,120,135,150],"Beat":[1215.6,1032.2,921.7,859.4,840,835.1,853.2],"Run":[1102.9,886.5,746.4,655.8,596.9,564.7,518.9],"BeatAngle":[42.8,42,42.8,41.4,40.9,40.7,41.8],"GybeAngle":[148,152,150.5,162,180,180,180],"WL":[1159.2,959.4,834.1,757.6,718.4,699.9,686],"CR":[989.5,803.2,704.2,647.7,614.3,594,572.2],"OC":[1153.6,900.2,752.5,666.7,624.4,593.1,543.1],"NS":[1044.7,843.1,734.6,671.9,634.2,611.1,586],"OL":[1116.2,923.6,815.1,753.8,722.7,706.6,697.2],"R52":[796.9,680.2,617.6,591.6,581.2,577.6,579],"R60":[753.3,644.5,598.1,576.8,565.2,560.4,559.4],"R75":[723,622.2,584.2,564,549,537.5,528],"R90":[729.8,623.6,577.4,552.9,539.1,527.4,503.9],"R110":[729.1,608.7,564.9,538.7,516.9,499.7,477.8],"R120":[750.3,617.8,568.6,540.6,515,493.4,466],"R135":[817.4,671.3,588.5,556.1,530.2,505,465.1],"R150":[955.2,767.9,647,584.3,555.6,531.9,487.7]}} +]} \ No newline at end of file diff --git a/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/orc/TestORCCertificateImporterJSON.java b/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/orc/TestORCCertificateImporterJSON.java index 224447d1736..a8c368b3edd 100644 --- a/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/orc/TestORCCertificateImporterJSON.java +++ b/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/orc/TestORCCertificateImporterJSON.java @@ -13,6 +13,11 @@ public class TestORCCertificateImporterJSON extends AbstractORCCertificateImport testSimpleLocalFileRead("GER2019.json", "GER20041179"); } + @Test + public void testReadingJSONWithSpecificBins() throws IOException, ParseException { + testSimpleLocalFileRead("03600000HUH.json", "03600000HUH"); + } + @FailIfNoValidOrcCertificates @Test public void testSimpleOnlineJSONFileRead() throws IOException, ParseException, InterruptedException { diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/orc/impl/ORCCertificatesCollectionJSON.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/orc/impl/ORCCertificatesCollectionJSON.java index fd1451eab32..ab0d9da765d 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/orc/impl/ORCCertificatesCollectionJSON.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/orc/impl/ORCCertificatesCollectionJSON.java @@ -63,6 +63,7 @@ public class ORCCertificatesCollectionJSON extends AbstractORCCertificatesCollec private static final String CIRCULAR_RANDOM = "CR"; private static final String NON_SPINNAKER = "NS"; private static final String WIND_SPEEDS = "WindSpeeds"; + private static final String WIND_ANGLES = "WindAngles"; /** * Pattern to recognize a true wind angle in the allowances section. Group 1 contains @@ -189,27 +190,28 @@ public class ORCCertificatesCollectionJSON extends AbstractORCCertificatesCollec } // Now read the various allowances, assuming we find values for the TWS "bins" identified above for (final Object aKey : allowances.keySet()) { - final JSONArray array = (JSONArray) allowances.get(aKey); + final JSONArray twaArray = (JSONArray) allowances.get(aKey); if (((String) aKey).equals(BEAT_ANGLE)) { - for (int i = 0; i < array.size(); i++) { + for (int i = 0; i < twaArray.size(); i++) { beatAngles.put(new KnotSpeedImpl(trueWindSpeedArray[i]), - new DegreeBearingImpl(((Number) array.get(i)).doubleValue())); + new DegreeBearingImpl(((Number) twaArray.get(i)).doubleValue())); } continue; } if (((String) aKey).equals(GYBE_ANGLE)) { - for (int i = 0; i < array.size(); i++) { + for (int i = 0; i < twaArray.size(); i++) { gybeAngles.put(new KnotSpeedImpl(trueWindSpeedArray[i]), - new DegreeBearingImpl(((Number) array.get(i)).doubleValue())); + new DegreeBearingImpl(((Number) twaArray.get(i)).doubleValue())); } continue; } Map twsMap = new HashMap<>(); // ignore true wind angles key - if (!twaPattern.matcher((String) aKey).lookingAt()) { - for (int i = 0; i < array.size(); i++) { + if (!aKey.equals(WIND_SPEEDS) && !aKey.equals(WIND_ANGLES) && + !twaPattern.matcher((String) aKey).lookingAt()) { + for (int i = 0; i < twaArray.size(); i++) { twsMap.put(new KnotSpeedImpl(trueWindSpeedArray[i]), - new SecondsDurationImpl(((Number) array.get(i)).doubleValue())); + new SecondsDurationImpl(((Number) twaArray.get(i)).doubleValue())); } } switch ((String) aKey) { diff --git a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/preferences/model/TrackedEventPreferences.java b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/preferences/model/TrackedEventPreferences.java index d4745dee645..f0c9803c86f 100644 --- a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/preferences/model/TrackedEventPreferences.java +++ b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/preferences/model/TrackedEventPreferences.java @@ -7,7 +7,6 @@ public class TrackedEventPreferences extends AbstractGenericSerializableSettings private static final long serialVersionUID = -290209497308646446L; - // TODO: setting preference title public static final String PREF_NAME = SailingPreferences.TRACKED_EVENTS_PREFERENCES; private transient SettingsList trackedEvents; diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/AbstractGenericSerializableSettings.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/AbstractGenericSerializableSettings.java index ceac4321bda..a50042c25d4 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/AbstractGenericSerializableSettings.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/AbstractGenericSerializableSettings.java @@ -91,7 +91,7 @@ public abstract class AbstractGenericSerializableSettings extends AbstractSettin public AbstractGenericSerializableSettings(String name, AbstractGenericSerializableSettings settings) { super(name, settings); value = (SettingsValue) settings.getValue(name); - if(value == null) { + if (value == null) { value = new SettingsValue(); settings.setValue(name, value); } @@ -103,18 +103,18 @@ public abstract class AbstractGenericSerializableSettings extends AbstractSettin */ protected void adoptValue(SettingsValue value) { this.value = value; - if(childSettings != null) { - for(Map.Entry entry : childSettings.entrySet()) { + if (childSettings != null) { + for (Map.Entry entry : childSettings.entrySet()) { Setting childSetting = entry.getValue(); - if(childSetting instanceof AbstractGenericSerializableSettings) { + if (childSetting instanceof AbstractGenericSerializableSettings) { Value childValue = value.getValue(entry.getKey()); - if(childValue != null) { + if (childValue != null) { ((AbstractGenericSerializableSettings) childSetting).adoptValue((SettingsValue) childValue); } } - if(childSetting instanceof SettingsList) { + if (childSetting instanceof SettingsList) { Value childValue = value.getValue(entry.getKey()); - if(childValue != null) { + if (childValue != null) { ((SettingsList) childSetting).adoptValue(); } } diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/CollectionSetting.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/CollectionSetting.java index 78f047ee039..034982acd4b 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/CollectionSetting.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/CollectionSetting.java @@ -5,6 +5,10 @@ public interface CollectionSetting extends Setting { void setValues(Iterable values); + /** + * Obtains a non-live copy of the values in this collection setting. Changes to this setting + * will not reflect in the iterable returned. + */ Iterable getValues(); void addValue(T value); diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/SettingsList.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/SettingsList.java index 601c43e2b8a..5b6e7cc47a8 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/SettingsList.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/SettingsList.java @@ -13,6 +13,13 @@ import com.sap.sse.common.settings.value.ValueListValue; public class SettingsList extends AbstractSetting implements SettingsListSetting { private SettingsFactory settingsFactory; + + /** + * All access to this list must be {@code synchronized}. Unfortunately, GWT does not offer + * {@link Collections#synchronizedList(List)} in its JRE emulation, so we have to + * make sure to consistently wrap all methods that access this list with a {@code synchronized} + * block that obtains this list's monitor. + */ private final List values = new ArrayList<>(); public SettingsList(String name, AbstractGenericSerializableSettings settings, SettingsFactory settingsFactory) { @@ -22,13 +29,15 @@ public class SettingsList extends } protected void adoptValue() { - values.clear(); - ValueListValue value = getValue(); - if(value != null) { - for(Value val : value.getValueObjects()) { - T childSettingsInstance = settingsFactory.newInstance(); - childSettingsInstance.adoptValue((SettingsValue)val); - values.add(childSettingsInstance); + synchronized (values) { + values.clear(); + ValueListValue value = getValue(); + if(value != null) { + for(Value val : value.getValueObjects()) { + T childSettingsInstance = settingsFactory.newInstance(); + childSettingsInstance.adoptValue((SettingsValue)val); + values.add(childSettingsInstance); + } } } } @@ -58,65 +67,80 @@ public class SettingsList extends @Override public boolean isDefaultValue() { - // explicit default values are possible to implement - // currently, empty is always default - return values.isEmpty(); + synchronized (values) { + // explicit default values are possible to implement + // currently, empty is always default + return values.isEmpty(); + } } @Override public void resetToDefault() { - this.values.clear(); - settings.setValue(settingName, null); + synchronized (values) { + this.values.clear(); + settings.setValue(settingName, null); + } } @Override public Iterable getValues() { - return Collections.unmodifiableCollection(values); + synchronized (values) { + final List result = new ArrayList<>(values); + return result; + } } @Override public void setValues(Iterable values) { - this.values.clear(); - ValueCollectionValue> valueObject = ensureValue(); - valueObject.clear(); - if (values != null) { - for (T value : values) { - this.values.add(value); - valueObject.addValue(value.getInnerValueObject()); + synchronized (values) { + this.values.clear(); + ValueCollectionValue> valueObject = ensureValue(); + valueObject.clear(); + if (values != null) { + for (T value : values) { + this.values.add(value); + valueObject.addValue(value.getInnerValueObject()); + } } } } @Override public void addValue(T value) { - ValueCollectionValue> valueObject = ensureValue(); - this.values.add(value); - valueObject.addValue(value.getInnerValueObject()); + synchronized (values) { + ValueCollectionValue> valueObject = ensureValue(); + this.values.add(value); + valueObject.addValue(value.getInnerValueObject()); + } } @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((values == null) ? 0 : values.hashCode()); - return result; + synchronized (values) { + final int prime = 31; + int result = 1; + result = prime * result + ((values == null) ? 0 : values.hashCode()); + return result; + } } @Override public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - @SuppressWarnings("rawtypes") - SettingsList other = (SettingsList) obj; - if (values == null) { - if (other.values != null) + synchronized (values) { + if (this == obj) + return true; + if (obj == null) return false; - } else if (!values.equals(other.values)) - return false; - return true; + if (getClass() != obj.getClass()) + return false; + @SuppressWarnings("rawtypes") + SettingsList other = (SettingsList) obj; + if (values == null) { + if (other.values != null) + return false; + } else if (!values.equals(other.values)) + return false; + return true; + } } } diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/ValueCollectionSetting.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/ValueCollectionSetting.java index 2f499594467..64a173d8c07 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/ValueCollectionSetting.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/ValueCollectionSetting.java @@ -3,6 +3,11 @@ package com.sap.sse.common.settings.generic; public interface ValueCollectionSetting extends HasValueSetting, CollectionSetting { void setDefaultValues(Iterable defaultValues); + + /** + * Obtains a non-live copy of the default values. Modification to this object's default values hence will + * not reflect into the iterable returned. + */ Iterable getDefaultValues(); boolean isValuesEmpty(); void setDiff(Iterable removedValues, Iterable addedValues); diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java index 0354eca632b..b95767959c4 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java @@ -21,7 +21,7 @@ public abstract class AbstractValueCollectionSetting ensureValue() { ValueCollectionValue result = getValue(); - if(result == null) { + if (result == null) { result = createValue(); settings.setValue(settingName, result); } @@ -48,10 +48,15 @@ public abstract class AbstractValueCollectionSetting getValues() { ValueCollectionValue value = getValue(); - if(emptyIsDefault && (value == null || value.isEmpty())) { - return Collections.unmodifiableCollection(getDefaultValuesCollectionInternal()); + if (emptyIsDefault && (value == null || value.isEmpty())) { + final D defaultVal = getDefaultValuesCollectionInternal(); + synchronized (defaultVal) { + final D result = createDefaultValuesCollection(); + result.addAll(defaultVal); + return result; + } } - if(value == null) { + if (value == null) { return Collections.emptyList(); } return value.getValues(getValueConverter()); @@ -78,33 +83,45 @@ public abstract class AbstractValueCollectionSetting value = getValue(); final D defaultVal = getDefaultValuesCollectionInternal(); - return ((emptyIsDefault || defaultVal.isEmpty()) && (value == null || value.isEmpty())) - || (value != null && value.size() == defaultVal.size() && defaultVal.containsAll(value.getValues(getValueConverter()))); + synchronized (defaultVal) { + return ((emptyIsDefault || defaultVal.isEmpty()) && (value == null || value.isEmpty())) + || (value != null && value.size() == defaultVal.size() && defaultVal.containsAll(value.getValues(getValueConverter()))); + } } @Override public final void setDefaultValues(Iterable defaultValues) { final boolean wasDefault = isDefaultValue(); final D defaultVal = getDefaultValuesCollectionInternal(); - defaultVal.clear(); - if(defaultValues != null) { - Util.addAll(defaultValues, defaultVal); - } - if(wasDefault) { - resetToDefault(); + synchronized (defaultVal) { + defaultVal.clear(); + if (defaultValues != null) { + Util.addAll(defaultValues, defaultVal); + } + if (wasDefault) { + resetToDefault(); + } } } @Override public Iterable getDefaultValues() { - return Collections.unmodifiableCollection(getDefaultValuesCollectionInternal()); + final D result = createDefaultValuesCollection(); + final D localDefaultValues = getDefaultValuesCollectionInternal(); + synchronized (localDefaultValues) { + result.addAll(localDefaultValues); + return result; + } } @Override @@ -135,7 +152,7 @@ public abstract class AbstractValueCollectionSetting value = getValue(); - if(value == null) { + if (value == null) { return "[]"; } return value.toString(); diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsListValue.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsListValue.java index decbc97a900..04c8871c40d 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsListValue.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsListValue.java @@ -3,21 +3,24 @@ package com.sap.sse.common.settings.value; import java.util.HashMap; public class SettingsListValue { - private HashMap values = new HashMap<>(); public SettingsListValue() { } public Value getValue(String name) { - return values.get(name); + synchronized (values) { + return values.get(name); + } } public void setValue(String name, Value value) { - if(value == null) { - values.remove(name); - } else { - values.put(name, value); + synchronized (values) { + if (value == null) { + values.remove(name); + } else { + values.put(name, value); + } } } } diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsValue.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsValue.java index 3b5f984465b..98e1ed5cbd5 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsValue.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/SettingsValue.java @@ -1,53 +1,71 @@ package com.sap.sse.common.settings.value; +import java.util.Collections; import java.util.HashMap; public class SettingsValue implements Value { private static final long serialVersionUID = -3407605464729611500L; + + /** + * All access to this map must be {@code synchronized}. Unfortunately, GWT does not offer + * {@link Collections#synchronizedMap(java.util.Map)} in its JRE emulation, so we have to + * make sure to consistently wrap all methods that access this map with a {@code synchronized} + * block that obtains this map's monitor. + */ private HashMap values = new HashMap<>(); public SettingsValue() { } public Value getValue(String name) { - return values.get(name); + synchronized (values) { + return values.get(name); + } } public void setValue(String name, Value value) { - if(value == null) { - values.remove(name); - } else { - values.put(name, value); + synchronized (values) { + if (value == null) { + values.remove(name); + } else { + values.put(name, value); + } } } @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((values == null) ? 0 : values.hashCode()); - return result; + synchronized (values) { + final int prime = 31; + int result = 1; + result = prime * result + ((values == null) ? 0 : values.hashCode()); + return result; + } } @Override public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - SettingsValue other = (SettingsValue) obj; - if (values == null) { - if (other.values != null) + synchronized (values) { + if (this == obj) + return true; + if (obj == null) return false; - } else if (!values.equals(other.values)) - return false; - return true; + if (getClass() != obj.getClass()) + return false; + SettingsValue other = (SettingsValue) obj; + if (values == null) { + if (other.values != null) + return false; + } else if (!values.equals(other.values)) + return false; + return true; + } } @Override public String toString() { - return values.toString(); + synchronized (values) { + return values.toString(); + } } } diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollection.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollection.java index 66f273a1e54..fb151da9c40 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollection.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollection.java @@ -3,6 +3,10 @@ package com.sap.sse.common.settings.value; import com.sap.sse.common.settings.generic.ValueConverter; public interface ValueCollection extends Value { + /** + * Obtains a non-live copy of the values in this collection. Modifications to this value collection + * will not reflect in the result of this method, and vice versa. + */ Iterable getValues(ValueConverter converter); void setValues(Iterable values, ValueConverter converter); diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollectionValue.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollectionValue.java index 72ec892fe92..cd5332ac609 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollectionValue.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/value/ValueCollectionValue.java @@ -7,6 +7,13 @@ import com.sap.sse.common.settings.generic.ValueConverter; public abstract class ValueCollectionValue> implements ValueCollection { private static final long serialVersionUID = -5820765644801217519L; + + /** + * All access to this collection must be {@code synchronized}. Unfortunately, GWT does not offer + * {@link Collections#synchronizedCollection(Collection)} in its JRE emulation, so we have to + * make sure to consistently wrap all methods that access this collection with a {@code synchronized} + * block that obtains this collection's monitor. + */ private C values; protected ValueCollectionValue(C values) { @@ -17,75 +24,99 @@ public abstract class ValueCollectionValue> implemen @Override public Collection getValues(ValueConverter converter) { - Collection result = emptyCollection(); - for (Value value : values) { - result.add(converter.fromValue(value)); + final Collection result = emptyCollection(); + synchronized (values) { + for (Value value : values) { + result.add(converter.fromValue(value)); + } + return result; + } + } + + public Iterable getValueObjects() { + final Collection result = emptyCollection(); + synchronized (values) { + result.addAll(values); } return result; } - public Iterable getValueObjects() { - return Collections.unmodifiableCollection(values); - } - @Override public void setValues(Iterable values, ValueConverter converter) { - clear(); - if(values != null) { - for (T value : values) { - addValue(value, converter); + synchronized (this.values) { + clear(); + if (values != null) { + for (T value : values) { + addValue(value, converter); + } } } } @Override public void clear() { - this.values.clear(); + synchronized (values) { + this.values.clear(); + } } public void addValue(T value, ValueConverter converter) { - this.values.add(converter.toValue(value)); + synchronized (values) { + addValue(converter.toValue(value)); + } } public void addValue(Value value) { - this.values.add(value); + synchronized (values) { + this.values.add(value); + } } public boolean isEmpty() { - return values.isEmpty(); + synchronized (values) { + return values.isEmpty(); + } } public int size() { - return values.size(); + synchronized (values) { + return values.size(); + } } @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((values == null) ? 0 : values.hashCode()); - return result; + synchronized (values) { + final int prime = 31; + int result = 1; + result = prime * result + ((values == null) ? 0 : values.hashCode()); + return result; + } } @Override public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - ValueCollectionValue other = (ValueCollectionValue) obj; - if (values == null) { - if (other.values != null) + synchronized (values) { + if (this == obj) + return true; + if (obj == null) return false; - } else if (!values.equals(other.values)) - return false; - return true; + if (getClass() != obj.getClass()) + return false; + ValueCollectionValue other = (ValueCollectionValue) obj; + if (values == null) { + if (other.values != null) + return false; + } else if (!values.equals(other.values)) + return false; + return true; + } } @Override public String toString() { - return values.toString(); + synchronized (values) { + return values.toString(); + } } } diff --git a/java/com.sap.sse.gwt/src/com/sap/sse/gwt/client/shared/components/CompositeSettings.java b/java/com.sap.sse.gwt/src/com/sap/sse/gwt/client/shared/components/CompositeSettings.java index 94d8249fda4..02a3f9c3375 100755 --- a/java/com.sap.sse.gwt/src/com/sap/sse/gwt/client/shared/components/CompositeSettings.java +++ b/java/com.sap.sse.gwt/src/com/sap/sse/gwt/client/shared/components/CompositeSettings.java @@ -12,7 +12,10 @@ import com.sap.sse.gwt.client.shared.perspective.PerspectiveCompositeSettings; /** * Groups settings for multiple {@link Component}s. This can be of particular interest when working with - * {@link Perspective}s and the perspective's {@link PerspectiveCompositeSettings composite settings}. + * {@link Perspective}s and the perspective's {@link PerspectiveCompositeSettings composite settings}.

+ * + * No synchronization is required here because when the constructor returns, the map used internally + * will no longer be modified. */ public class CompositeSettings extends AbstractSettings implements SettingsMap { private final Map settingsPerComponentId;