From 1b1a407423f36a50dfcf20a0923b99aee1743600 Mon Sep 17 00:00:00 2001 From: JJB8035 Date: Thu, 20 Feb 2020 15:27:03 -0500 Subject: [PATCH 1/8] Remove unused imports --- app/controllers/Application.java | 25 +++++++++---------------- app/models/ApexClass.java | 1 - 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/app/controllers/Application.java b/app/controllers/Application.java index 6dbe0a7..da67ddd 100644 --- a/app/controllers/Application.java +++ b/app/controllers/Application.java @@ -1,30 +1,23 @@ package controllers; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.HashMap; -import java.util.Map; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; - -import models.ApexList; import models.ApexType; import models.TypeFactory; - import org.codehaus.jackson.map.ObjectMapper; - import play.data.validation.Required; import play.data.validation.Validation; import play.mvc.Controller; -import play.mvc.Http; -import play.mvc.Scope; -import play.mvc.results.RenderTemplate; -import play.mvc.results.Result; import play.templates.Template; import play.templates.TemplateLoader; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + public class Application extends Controller { public static void index(String json, String className) { diff --git a/app/models/ApexClass.java b/app/models/ApexClass.java index 7d4c378..c0be453 100644 --- a/app/models/ApexClass.java +++ b/app/models/ApexClass.java @@ -1,6 +1,5 @@ package models; -import java.io.IOException; import java.util.Map; import java.util.Objects; From dd8665f97fa0272097d8f7df3669f9d2c09ed204 Mon Sep 17 00:00:00 2001 From: Joseph Bleau Date: Fri, 21 Feb 2020 00:24:25 -0500 Subject: [PATCH 2/8] When merging two ApexClass objects we prefer members we willingly overwrite a members type with a primitive if it's currently Object, additionally, when encountering a member during a merge operation that is an ApexClass we recursively call merge on all of that ApexClass' members. --- app/models/ApexClass.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/ApexClass.java b/app/models/ApexClass.java index c0be453..04d6d20 100644 --- a/app/models/ApexClass.java +++ b/app/models/ApexClass.java @@ -64,7 +64,11 @@ boolean membersEqual(Map other) { void mergeFields(ApexClass other) { for (ApexMember key : other.getMembers().keySet() ) { - if (members.get(key) == null) { + if (members.get(key) instanceof ApexClass && other.getMembers().get(key) instanceof ApexClass) { + ((ApexClass) members.get(key)).mergeFields((ApexClass) other.getMembers().get(key)); + } + + if (members.get(key) == null || members.get(key) == ApexPrimitive.OBJECT) { members.put(key, other.getMembers().get(key)); } } From 2f95c8b025889f7be0b5b4c0e9bd89be94b6c08f Mon Sep 17 00:00:00 2001 From: Joseph Bleau Date: Fri, 21 Feb 2020 02:55:10 -0500 Subject: [PATCH 3/8] When merging objects within a list of object types be sure to account for the fact that those members might also contain lists that need to be merged and recursed on. (WIP) --- app/models/ApexClass.java | 30 ++++++++++++++++++++++++++++-- app/models/TypeFactory.java | 7 ++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/models/ApexClass.java b/app/models/ApexClass.java index 04d6d20..48e460c 100644 --- a/app/models/ApexClass.java +++ b/app/models/ApexClass.java @@ -1,7 +1,9 @@ package models; +import java.util.HashSet; import java.util.Map; import java.util.Objects; +import java.util.Set; public class ApexClass extends ApexType { @@ -62,15 +64,39 @@ boolean membersEqual(Map other) { return members.equals(other); } - void mergeFields(ApexClass other) { + Set mergeFields(ApexClass other) { + Set classesToRemove = new HashSet<>(); + for (ApexMember key : other.getMembers().keySet() ) { + // If our member is an array and the other member is also an array check the item types of + // each array and if they're both ApexClass' then merge them. + if (members.get(key) instanceof ApexList && other.getMembers().get(key) instanceof ApexList) { + ApexList ourList = (ApexList) members.get(key); + ApexList otherList = (ApexList) other.getMembers().get(key); + + if (ourList.itemType instanceof ApexClass && otherList.itemType instanceof ApexClass) { + ApexClass itemType = (ApexClass) ourList.itemType; + ApexClass otherType = (ApexClass) otherList.itemType; + + classesToRemove.addAll(itemType.mergeFields(otherType)); + } + } + + // Cover case where two members of the same name exist, and both are classes, and so we want to + // recursively merge the members of those classes. if (members.get(key) instanceof ApexClass && other.getMembers().get(key) instanceof ApexClass) { - ((ApexClass) members.get(key)).mergeFields((ApexClass) other.getMembers().get(key)); + classesToRemove.addAll(((ApexClass) members.get(key)).mergeFields((ApexClass) other.getMembers().get(key))); } + // Merge a member if it's null, or if the existing member is an Object, because + // that Object may have been originally just from a null value, and is not necessarily + // determinant of it's final type. if (members.get(key) == null || members.get(key) == ApexPrimitive.OBJECT) { members.put(key, other.getMembers().get(key)); + classesToRemove.add(other.toString()); } } + + return classesToRemove; } } diff --git a/app/models/TypeFactory.java b/app/models/TypeFactory.java index 4062edd..9d4827d 100644 --- a/app/models/TypeFactory.java +++ b/app/models/TypeFactory.java @@ -103,10 +103,11 @@ ApexType typeOfCollection(String propertyName, Collection col) { if (itemType instanceof ApexClass && thisItemType instanceof ApexClass) { ApexClass apexClass = (ApexClass)itemType; ApexClass thisApexClass = (ApexClass)thisItemType; - - apexClass.mergeFields(thisApexClass); - classes.remove(thisApexClass.toString()); + Set classesToRemove = apexClass.mergeFields(thisApexClass); + for (String k : classesToRemove) { + classes.remove(k); + } } else if (itemType instanceof ApexPrimitive && thisItemType instanceof ApexPrimitive) { ApexPrimitive a = (ApexPrimitive)itemType; ApexPrimitive b = (ApexPrimitive)thisItemType; From cbb5d02909f015535626f5c07273e0c0e385ff41 Mon Sep 17 00:00:00 2001 From: Joseph Bleau Date: Fri, 21 Feb 2020 13:06:14 -0500 Subject: [PATCH 4/8] Add IntelliJ extensions to .gitignore --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index d00427d..7f1eb72 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,6 @@ /eclipse/classes /integration-test/__pycache__ *.pyc - +*.iml +*.ipr +*.iws From 434005717bab8759577a598d8187536a30b6aa14 Mon Sep 17 00:00:00 2001 From: Joseph Bleau Date: Fri, 21 Feb 2020 14:01:08 -0500 Subject: [PATCH 5/8] Oof ... merge in both directions. This is because now that we're overwriting Object primitives with complex types it's possible that we have information about a complex member but are only merging in the direction that will cause us to hold on to our primitive. --- app/models/TypeFactory.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/TypeFactory.java b/app/models/TypeFactory.java index 9d4827d..675ac86 100644 --- a/app/models/TypeFactory.java +++ b/app/models/TypeFactory.java @@ -104,10 +104,11 @@ ApexType typeOfCollection(String propertyName, Collection col) { ApexClass apexClass = (ApexClass)itemType; ApexClass thisApexClass = (ApexClass)thisItemType; - Set classesToRemove = apexClass.mergeFields(thisApexClass); - for (String k : classesToRemove) { - classes.remove(k); - } + // Merge in both directions (for object-overwrite) + thisApexClass.mergeFields(apexClass); + apexClass.mergeFields(thisApexClass); + + // TODO: Re-implement class removal logic } else if (itemType instanceof ApexPrimitive && thisItemType instanceof ApexPrimitive) { ApexPrimitive a = (ApexPrimitive)itemType; ApexPrimitive b = (ApexPrimitive)thisItemType; From a0d92f168a03685ab73c4bbc59397a6e68af5b6f Mon Sep 17 00:00:00 2001 From: Joseph Bleau Date: Fri, 21 Feb 2020 14:02:08 -0500 Subject: [PATCH 6/8] Add some exit-early scenarios to mergeFields. Update membersEqual so that it does a deep comparison of all members and all of their members members etc. --- app/models/ApexClass.java | 49 +++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/app/models/ApexClass.java b/app/models/ApexClass.java index 48e460c..2cf5e48 100644 --- a/app/models/ApexClass.java +++ b/app/models/ApexClass.java @@ -13,24 +13,24 @@ public class ApexClass extends ApexType { this.className = className; this.members = members; } - + private final String className; private final Map members; - + public Map getMembers() { return members; } - + @Override public String getParserExpr(String parserName) { return String.format("new %s(%s)", className, parserName); } - + @Override public String additionalMethods() { return ""; } - + public boolean shouldGenerateExplictParse() { for (ApexMember m : members.keySet()) { if (m.shouldGenerateExplictParse()) { @@ -39,7 +39,7 @@ public boolean shouldGenerateExplictParse() { } return false; } - + @Override public String toString() { return className; @@ -56,17 +56,42 @@ public boolean equals(Object obj) { if (obj == null) return false; if (getClass() != obj.getClass()) return false; ApexClass other = (ApexClass) obj; - return className.equals(other.className); + return membersEqual(other.members); } - - /** @return true if this map of members equals our map of members */ - boolean membersEqual(Map other) { - return members.equals(other); + + /** @return true if every member of our type is equal to every member of the given type and if + all members in our type are present in the other type. Otherwise returns false. **/ + boolean membersEqual(Map otherMembers) { + for (Map.Entry entry : this.members.entrySet()) { + ApexMember memberName = entry.getKey(); + ApexType member = entry.getValue(); + + if (otherMembers.get(memberName) == null) { + return false; + } else { + ApexType otherMember = otherMembers.get(memberName); + if (!member.equals(otherMember)) { + return false; + } + } + } + + return true; } - + Set mergeFields(ApexClass other) { Set classesToRemove = new HashSet<>(); + // Nothing needs to be done if the class being merged is the same as our class already. + if (this.equals(other)) { + return classesToRemove; + } + + // If the object being merged has zero members then we should just discard it. + if (other.getMembers().size() == 0) { + classesToRemove.add(other.toString()); + } + for (ApexMember key : other.getMembers().keySet() ) { // If our member is an array and the other member is also an array check the item types of // each array and if they're both ApexClass' then merge them. From 082f8c716698afcd31f6bbda0136c26421c4d5a8 Mon Sep 17 00:00:00 2001 From: JJB8035 Date: Mon, 24 Feb 2020 13:48:11 -0500 Subject: [PATCH 7/8] Fix class removal logic by updating the equals members of ApexPrimitive, ApexClass, and ApexList. The missing component was the fact that an ApexPrimitive.OBJECT type should now be considered "equal" to all other types. Effectively, it's being used as an "UnknownType" property, and because equality is used to determine "same type"-edness and overwriting in merges it must be equal to all other types. I am going to probably use a different name overall in a upcoming change. ApexPrimitive.UnknownType is a good candidate. --- app/models/ApexClass.java | 3 ++- app/models/ApexList.java | 7 +++++++ app/models/ApexPrimitive.java | 4 +++- app/models/TypeFactory.java | 15 ++++++++++----- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/app/models/ApexClass.java b/app/models/ApexClass.java index 2cf5e48..c9e1627 100644 --- a/app/models/ApexClass.java +++ b/app/models/ApexClass.java @@ -54,7 +54,8 @@ public int hashCode() { public boolean equals(Object obj) { if (this == obj) return true; if (obj == null) return false; - if (getClass() != obj.getClass()) return false; + if(obj.toString().equals("Object")) return true; + if (!(obj instanceof ApexClass)) return false; ApexClass other = (ApexClass) obj; return membersEqual(other.members); } diff --git a/app/models/ApexList.java b/app/models/ApexList.java index 814ef69..a228e46 100644 --- a/app/models/ApexList.java +++ b/app/models/ApexList.java @@ -48,8 +48,15 @@ public int hashCode() { public boolean equals(Object obj) { if (this == obj) return true; if (obj == null) return false; + if (obj.toString().equals("Object")) return true; if (getClass() != obj.getClass()) return false; + ApexList other = (ApexList) obj; + + if (itemType instanceof ApexClass && other.itemType instanceof ApexClass) { + return ((ApexClass) itemType).membersEqual(((ApexClass) other.itemType).getMembers()); + } + return itemType.equals(other.itemType); } } \ No newline at end of file diff --git a/app/models/ApexPrimitive.java b/app/models/ApexPrimitive.java index 2b89650..800a4ba 100644 --- a/app/models/ApexPrimitive.java +++ b/app/models/ApexPrimitive.java @@ -49,8 +49,10 @@ boolean canBePromotedTo(ApexPrimitive other) { public boolean equals(Object obj) { if (this == obj) return true; if (obj == null) return false; + if (this.type.equals("Object")) return true; if (getClass() != obj.getClass()) return false; + ApexPrimitive other = (ApexPrimitive) obj; - return type.equals(other.type); + return other.type.equals("Object") || type.equals(other.type); } } diff --git a/app/models/TypeFactory.java b/app/models/TypeFactory.java index 675ac86..5988594 100644 --- a/app/models/TypeFactory.java +++ b/app/models/TypeFactory.java @@ -106,9 +106,12 @@ ApexType typeOfCollection(String propertyName, Collection col) { // Merge in both directions (for object-overwrite) thisApexClass.mergeFields(apexClass); - apexClass.mergeFields(thisApexClass); + Set classesToRemove = apexClass.mergeFields(thisApexClass); + + for (String className : classesToRemove) { + classes.remove(className); + } - // TODO: Re-implement class removal logic } else if (itemType instanceof ApexPrimitive && thisItemType instanceof ApexPrimitive) { ApexPrimitive a = (ApexPrimitive)itemType; ApexPrimitive b = (ApexPrimitive)thisItemType; @@ -128,13 +131,15 @@ ApexType typeOfCollection(String propertyName, Collection col) { /** @return an ApexClass for this map */ ApexType typeOfMap(String propertyName, Map o) { Map members = makeMembers(o); + String newClassName = getClassName(propertyName); + ApexClass newClass = new ApexClass(newClassName, members); + // see if any existing classes have the same member set for (ApexClass cls : classes.values()) { - if (cls.membersEqual(members)) + if (newClass.membersEqual(cls.getMembers())) return cls; } - String newClassName = getClassName(propertyName); - ApexClass newClass = new ApexClass(newClassName, members); + classes.put(newClassName, newClass); return newClass; } From 17726aa6153eed90b9fa99b3421e9eca1423f67c Mon Sep 17 00:00:00 2001 From: JJB8035 Date: Mon, 24 Feb 2020 14:27:43 -0500 Subject: [PATCH 8/8] No longer first check if two types are "equal" before merging them (when encountered in a list together), as two types may be equal by the fact that one of their members has the same name as the other, but is of UnknownType (Object) and the other is already mapped. When this happens we still want to merge them, and so equality cannot be used to prefilter mergable types. We should implement a method that determines "true equality" aka "all members are the same name, and the same type" as an optimization, but merging always is not a huge performance problem... for me... Lastly, for simmilar reasons, remove member list from hash value of ApexClass as we were getting duplicates in the type set (same class, but one with an UnknownType member and another which was resolved.) --- app/models/ApexClass.java | 7 +------ app/models/ApexPrimitive.java | 2 +- app/models/TypeFactory.java | 37 ++++++++++++++++++++--------------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/models/ApexClass.java b/app/models/ApexClass.java index c9e1627..6fd495b 100644 --- a/app/models/ApexClass.java +++ b/app/models/ApexClass.java @@ -47,7 +47,7 @@ public String toString() { @Override public int hashCode() { - return Objects.hash(className, members); + return Objects.hash(className); } @Override @@ -83,11 +83,6 @@ boolean membersEqual(Map otherMembers) { Set mergeFields(ApexClass other) { Set classesToRemove = new HashSet<>(); - // Nothing needs to be done if the class being merged is the same as our class already. - if (this.equals(other)) { - return classesToRemove; - } - // If the object being merged has zero members then we should just discard it. if (other.getMembers().size() == 0) { classesToRemove.add(other.toString()); diff --git a/app/models/ApexPrimitive.java b/app/models/ApexPrimitive.java index 800a4ba..66d4f96 100644 --- a/app/models/ApexPrimitive.java +++ b/app/models/ApexPrimitive.java @@ -28,7 +28,7 @@ public String toString() { @Override public int hashCode() { - return Objects.hash(type, parserMethod); + return Objects.hash(type); } @Override diff --git a/app/models/TypeFactory.java b/app/models/TypeFactory.java index 5988594..0f5ec05 100644 --- a/app/models/TypeFactory.java +++ b/app/models/TypeFactory.java @@ -88,19 +88,21 @@ private ApexType typeOfObjectImpl(String propertyName, Object o) { throw new RuntimeException("Unexpected type " + o.getClass() + " in TypeFactory.typeOfObject()"); } - + /** @return a concrete type if all the list items are of the same type, Object, otherwise */ ApexType typeOfCollection(String propertyName, Collection col) { if (col == null || col.size() == 0) { return typeOfObject(propertyName, Collections.EMPTY_MAP); } + ApexType itemType = null; + for (Object o : col) { ApexType thisItemType = typeOfObject(propertyName, o); + if (itemType == null) { itemType = thisItemType; - } else if (!itemType.equals(thisItemType)) { - if (itemType instanceof ApexClass && thisItemType instanceof ApexClass) { + } else if (itemType instanceof ApexClass && thisItemType instanceof ApexClass) { ApexClass apexClass = (ApexClass)itemType; ApexClass thisApexClass = (ApexClass)thisItemType; @@ -108,23 +110,24 @@ ApexType typeOfCollection(String propertyName, Collection col) { thisApexClass.mergeFields(apexClass); Set classesToRemove = apexClass.mergeFields(thisApexClass); + classesToRemove.remove(itemType.toString()); for (String className : classesToRemove) { classes.remove(className); } - } else if (itemType instanceof ApexPrimitive && thisItemType instanceof ApexPrimitive) { - ApexPrimitive a = (ApexPrimitive)itemType; - ApexPrimitive b = (ApexPrimitive)thisItemType; - if (a.canBePromotedTo(b)) { - itemType = b; - } else if (b.canBePromotedTo(a)) { - continue; - } - } else { - throw new RuntimeException("Can't add an " + o.getClass() + " to a collection of " + itemType.getClass()); - } + } else if (itemType instanceof ApexPrimitive && thisItemType instanceof ApexPrimitive) { + ApexPrimitive a = (ApexPrimitive)itemType; + ApexPrimitive b = (ApexPrimitive)thisItemType; + if (a.canBePromotedTo(b)) { + itemType = b; + } else if (b.canBePromotedTo(a)) { + continue; + } + } else { + throw new RuntimeException("Can't add an " + o.getClass() + " to a collection of " + itemType.getClass()); } } + return itemType; } @@ -136,8 +139,10 @@ ApexType typeOfMap(String propertyName, Map o) { // see if any existing classes have the same member set for (ApexClass cls : classes.values()) { - if (newClass.membersEqual(cls.getMembers())) - return cls; + if (newClass.membersEqual(cls.getMembers())) { + cls.mergeFields(newClass); + return cls; + } } classes.put(newClassName, newClass);