-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
De-duplicate identical classes and avoid __1
#1655
base: master
Are you sure you want to change the base?
Conversation
@@ -47,6 +50,7 @@ public class RuleFactory { | |||
private GenerationConfig generationConfig; | |||
private Annotator annotator; | |||
private SchemaStore schemaStore; | |||
private Map<Class<?>, Map<String, ?>> dedupeCache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Track duplicates in each RuleFactory
instance
return new ObjectRule(this, new ParcelableHelper(), reflectionHelper); | ||
Rule<JPackage, JType> rule = new ObjectRule(this, new ParcelableHelper(), reflectionHelper); | ||
if (generationConfig.isUseDeduplication()) { | ||
rule = new DeduplicateRule<>(dedupeCache, rule); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap specific Rule
s that require de-duplication.
|
||
public DeduplicateRule(Map<Class<?>, Map<String, ?>> dedupeCacheByRule, Rule<T, R> rule) { | ||
// noinspection unchecked map is populated by us and guaranteed to be of the correct type | ||
Map<String, R> dedupeCache = (Map<String, R>) dedupeCacheByRule.get(rule.getClass()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each Rule
has its own individual cache
* @return same {@code R} instance if a previous schema with the same hash code was already processed | ||
*/ | ||
@Override | ||
public R apply(String nodeName, JsonNode node, JsonNode parent, T generatableType, Schema currentSchema) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performs de-duplication here during rule transformation of Schema
to result.
@@ -75,4 +80,13 @@ public boolean isGenerated() { | |||
return javaType != null; | |||
} | |||
|
|||
public String calculateHash() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using SHA-256 of the compact JSON string for de-duplication.
Hello,
Thank you very much for your amazing project!
I know you have previously decided (in #1082) not to support de-duplication of identical classes and instead suggest to use
$ref
s, but please re-consider.This seems to be an arguably common request: #1562 #1559 #1151 #1192 #1081 #784 #114 #112
Mainly this will solve the case where it's not possible to modify the input and replace
$ref
s (my case), but it'll also make it friendlier for others not having to refactor their input to use$ref
s.Take a look at the implementation which is fairly simple. I'd appreciate any feedback on the implementation, even if you don't decide to go ahead with it.
If I still can't change your mind, no problem, I will maintain this change in my fork.