Re: Your commit b5336b0c introduced a bug

Kieran Kelleher <[email protected]> Mon, 22 Jul 2013 12:59:22 -0400
Newsgroups gmane.comp.web.webobjects.wonder-disc
Message-ID <[email protected]>
Thanks Johann.

On Jul 22, 2013, at 12:15 PM, Johann Werner <[email protected]> wrote:

> Hi Kieran,
> 
> I took out that code change and moved it into my own frameworks for now. When I have time I will revisit that code.
> 
> jw
> 
> 
> Am 22.07.2013 um 16:43 schrieb Kieran Kelleher <[email protected]>:
> 
>> Hi Johann,
>> 
>> Evidently a bug was introduced in ERXEOAccessUtilities.java by your commit b5336b0c?
>> 
>> By replacing the recursive call to entityHierarchyForEntity(...), the entire method returns an empty array. I suggest you revert the method entityHierarchyForEntity to what it was before your commit. Also either remove or fix and test your new method that you added, namely allSubEntitiesForEntity(...). Your commit comment gave no indication of this change - was this change to ERXEOAccessUtilities accidentally included?
>> 
>> Everyone, please do not change core wonder utility methods without testing your changes..
>> 
>> Thanks, Kieran
>> 
>> -------------------------------------
>> 
>> $ git show b5336b0c
>> 
>> commit b5336b0c9f0caf707e45931dd0ab32c7cbf9efcd
>> Author: Johann Werner <[email protected]>
>> Date:   Mon Nov 19 05:53:26 2012
>> 
>>   move methods to appropriate class and deprecate old ones
>> 
>> diff --git a/Frameworks/Core/ERExtensions/Sources/er/extensions/eof/ERXEOAccessUtilities.java b/Frameworks/Core/ERExtensions/Sources/er/extensions/eof/ERXEOAccessUtilities.java
>> index cedacb1..472bfc5 100644
>> --- a/Frameworks/Core/ERExtensions/Sources/er/extensions/eof/ERXEOAccessUtilities.java
>> +++ b/Frameworks/Core/ERExtensions/Sources/er/extensions/eof/ERXEOAccessUtilities.java
>> @@ -2315,14 +2315,38 @@ public class ERXEOAccessUtilities {
>> 	 */
>> 	public static NSArray<EOEntity> entityHierarchyForEntity(EOEditingContext ec, EOEntity rootEntity) {
>> 		NSMutableArray<EOEntity> entities = new NSMutableArray<EOEntity>();
>> -	
>> -		if (!rootEntity.isAbstractEntity()) {
>> -			entities.add(rootEntity);
>> +
>> +		if (rootEntity != null) {
>> +			if (!rootEntity.isAbstractEntity()) {
>> +				entities.add(rootEntity);
>> +			}
>> +			for (EOEntity subEntity : rootEntity.subEntities()) {
>> +				entities.addAll(allSubEntitiesForEntity(subEntity, false));
>> +			}
>> 		}
>> -		@SuppressWarnings("unchecked")
>> -		NSArray<EOEntity> subEntities = rootEntity.subEntities();
>> -		for (EOEntity subEntity : subEntities) {
>> -			entities.addAll(entityHierarchyForEntity(ec, subEntity));
>> +		return entities.immutableClone();
>> +	}
>> +
>> +	/**
>> +	 * Utility method used to find all of the sub entities
>> +	 * for a given entity.
>> +	 * @param rootEntity to walk all of the <code>subEntities</code>
>> +	 *            relationships
>> +	 * @param includeAbstracts determines if abstract entities should
>> +	 *            be included in the returned array
>> +	 * @return all of the sub-entities for a given entity.
>> +	 */
>> +	public static NSArray<EOEntity> allSubEntitiesForEntity(EOEntity rootEntity, boolean includeAbstracts) {
>> +		NSMutableArray<EOEntity> entities = new NSMutableArray<EOEntity>();
>> +		if (rootEntity != null) {
>> +			for (EOEntity subEntity : rootEntity.subEntities()) {
>> +				if (!subEntity.isAbstractEntity() || includeAbstracts) {
>> +					entities.addObject(subEntity);
>> +				}
>> +				if (subEntity.subEntities().count() > 0) {
>> +					entities.addAll(allSubEntitiesForEntity(subEntity, includeAbstracts));
>> +				}
>> +			}
>> 		}
>> 		return entities.immutableClone();
>> 	}
>> diff --git a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXTimestampUtilities.java b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXTimestampUtilities.java
>> index 9b0d208..cd04e96 100644
>> --- a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXTimestampUtilities.java
>> +++ b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXTimestampUtilities.java
>> @@ -9,11 +9,14 @@ import java.util.GregorianCalendar;
>> import com.webobjects.foundation.NSComparator;
>> import com.webobjects.foundation.NSTimeZone;
>> import com.webobjects.foundation.NSTimestamp;
>> +import com.webobjects.foundation.NSTimestampFormatter;
>> 
>> /**
>> * Collection of {@link com.webobjects.foundation.NSTimestamp NSTimestamp} utilities.
>> */
>> public class ERXTimestampUtilities extends Object {
>> +    /** caches date formatter the first time it is used */
>> +    private static NSTimestampFormatter _gregorianDateFormatterForJavaDate;
>> 
>>    /**
>>     * Calculates a timestamp given a string. Currently supports
>> @@ -525,4 +528,16 @@ public class ERXTimestampUtilities extends Object {
>>    public static int yearOfCommonEra(NSTimestamp t) {
>>        return calendarForTimestamp(t).get(Calendar.YEAR);
>>    }
>> +
>> +    /**
>> +     * Utility method to return a standard timestamp
>> +     * formatter for the default string representation
>> +     * of java dates.
>> +     * @return timestamp formatter for java dates.
>> +     */
>> +    public static NSTimestampFormatter gregorianDateFormatterForJavaDate() {
>> +        if (_gregorianDateFormatterForJavaDate == null)
>> +            _gregorianDateFormatterForJavaDate = new NSTimestampFormatter("%a %b %d %H:%M:%S %Z %Y");
>> +        return _gregorianDateFormatterForJavaDate;
>> +    }
>> }
>> diff --git a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXUtilities.java b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXUtilities.java
>> index 94ee4a9..7987f85 100644
>> --- a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXUtilities.java
>> +++ b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXUtilities.java
>> @@ -333,7 +333,9 @@ public class ERXUtilities {
>>     * @param includeAbstracts determines if abstract entities should
>>     *		be included in the returned array
>>     * @return all of the sub-entities for a given entity.
>> +     * @deprecated user {@link ERXEOAccessUtilities#allSubEntitiesForEntity(EOEntity, boolean)} instead
>>     */
>> +    @Deprecated
>>    public static NSArray allSubEntitiesForEntity(EOEntity entity, boolean includeAbstracts) {
>>        NSMutableArray entities = new NSMutableArray();
>>        if (entity != null) {
>> @@ -353,7 +355,9 @@ public class ERXUtilities {
>>     * find the root entity.
>>     * @param entity to find the root parent
>>     * @return root parent entity
>> +     * @deprecated use {@link ERXEOAccessUtilities#rootEntityForEntity(EOEntity)} instead
>>     */
>> +    @Deprecated
>>    public static EOEntity rootParentEntityForEntity(EOEntity entity) {
>>        EOEntity root = entity;
>>        while (root!=null && root.parentEntity() != null)
>> @@ -361,14 +365,16 @@ public class ERXUtilities {
>>        return root;
>>    }
>>    /** caches date formatter the first time it is used */
>> +    @Deprecated
>>    private static NSTimestampFormatter _gregorianDateFormatterForJavaDate;
>>    /**
>>     * Utility method to return a standard timestamp
>>     * formatter for the default string representation
>>     * of java dates.
>>     * @return timestamp formatter for java dates.
>> +     * @deprecated use {@link ERXTimestampUtilities#gregorianDateFormatterForJavaDate()} instead
>>     */
>> -    // MOVEME: Should move to ERXTimestampUtilities
>> +    @Deprecated
>>    public static NSTimestampFormatter gregorianDateFormatterForJavaDate() {
>>        if (_gregorianDateFormatterForJavaDate == null)
>>            _gregorianDateFormatterForJavaDate = new NSTimestampFormatter("%a %b %d %H:%M:%S %Z %Y");
>> 
>> 
> 


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk