SubmittedTogether: Reuse ChangeData instances
Teach ChangeJson to format a list of ChangeDatas so we don't have to go ChangeData -> Change.Id -> ChangeData and can reuse some stored fields in the index. Change-Id: I34eb564fe730baa71eeec64c36a593fb493186d8
This commit is contained in:
		| @@ -279,23 +279,13 @@ public class ChangeJson { | |||||||
|   public List<List<ChangeInfo>> formatQueryResults(List<QueryResult> in) |   public List<List<ChangeInfo>> formatQueryResults(List<QueryResult> in) | ||||||
|       throws OrmException { |       throws OrmException { | ||||||
|     accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); |     accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); | ||||||
|     Iterable<ChangeData> all = FluentIterable.from(in) |     ensureLoaded(FluentIterable.from(in) | ||||||
|         .transformAndConcat(new Function<QueryResult, List<ChangeData>>() { |         .transformAndConcat(new Function<QueryResult, List<ChangeData>>() { | ||||||
|           @Override |           @Override | ||||||
|           public List<ChangeData> apply(QueryResult in) { |           public List<ChangeData> apply(QueryResult in) { | ||||||
|             return in.changes(); |             return in.changes(); | ||||||
|           } |           } | ||||||
|         }); |         })); | ||||||
|     ChangeData.ensureChangeLoaded(all); |  | ||||||
|     if (has(ALL_REVISIONS)) { |  | ||||||
|       ChangeData.ensureAllPatchSetsLoaded(all); |  | ||||||
|     } else if (has(CURRENT_REVISION) || has(MESSAGES)) { |  | ||||||
|       ChangeData.ensureCurrentPatchSetLoaded(all); |  | ||||||
|     } |  | ||||||
|     if (has(REVIEWED) && userProvider.get().isIdentifiedUser()) { |  | ||||||
|       ChangeData.ensureReviewedByLoadedForOpenChanges(all); |  | ||||||
|     } |  | ||||||
|     ChangeData.ensureCurrentApprovalsLoaded(all); |  | ||||||
|  |  | ||||||
|     List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size()); |     List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size()); | ||||||
|     Map<Change.Id, ChangeInfo> out = Maps.newHashMap(); |     Map<Change.Id, ChangeInfo> out = Maps.newHashMap(); | ||||||
| @@ -310,6 +300,31 @@ public class ChangeJson { | |||||||
|     return res; |     return res; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public List<ChangeInfo> formatChangeDatas(Collection<ChangeData> in) | ||||||
|  |       throws OrmException { | ||||||
|  |     accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); | ||||||
|  |     ensureLoaded(in); | ||||||
|  |     List<ChangeInfo> out = new ArrayList<>(in.size()); | ||||||
|  |     for (ChangeData cd : in) { | ||||||
|  |       out.add(format(cd)); | ||||||
|  |     } | ||||||
|  |     accountLoader.fill(); | ||||||
|  |     return out; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |   private void ensureLoaded(Iterable<ChangeData> all) throws OrmException { | ||||||
|  |     ChangeData.ensureChangeLoaded(all); | ||||||
|  |     if (has(ALL_REVISIONS)) { | ||||||
|  |       ChangeData.ensureAllPatchSetsLoaded(all); | ||||||
|  |     } else if (has(CURRENT_REVISION) || has(MESSAGES)) { | ||||||
|  |       ChangeData.ensureCurrentPatchSetLoaded(all); | ||||||
|  |     } | ||||||
|  |     if (has(REVIEWED) && userProvider.get().isIdentifiedUser()) { | ||||||
|  |       ChangeData.ensureReviewedByLoadedForOpenChanges(all); | ||||||
|  |     } | ||||||
|  |     ChangeData.ensureCurrentApprovalsLoaded(all); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   private boolean has(ListChangesOption option) { |   private boolean has(ListChangesOption option) { | ||||||
|     return options.contains(option); |     return options.contains(option); | ||||||
|   } |   } | ||||||
|   | |||||||
| @@ -72,34 +72,34 @@ public class SubmittedTogether implements RestReadView<ChangeResource> { | |||||||
|       ResourceConflictException, Exception { |       ResourceConflictException, Exception { | ||||||
|     try { |     try { | ||||||
|       Change c = resource.getChange(); |       Change c = resource.getChange(); | ||||||
|       List<Change.Id> ids; |       List<ChangeData> cds; | ||||||
|       if (c.getStatus().isOpen()) { |       if (c.getStatus().isOpen()) { | ||||||
|         ids = getForOpenChange(c); |         cds = getForOpenChange(c); | ||||||
|       } else if (c.getStatus().asChangeStatus() == ChangeStatus.MERGED) { |       } else if (c.getStatus().asChangeStatus() == ChangeStatus.MERGED) { | ||||||
|         ids = getForMergedChange(c); |         cds = getForMergedChange(c); | ||||||
|       } else { |       } else { | ||||||
|         ids = getForAbandonedChange(); |         cds = getForAbandonedChange(); | ||||||
|       } |       } | ||||||
|       if (ids.size() <= 1) { |       if (cds.size() <= 1) { | ||||||
|         ids = Collections.emptyList(); |         cds = Collections.emptyList(); | ||||||
|       } |       } | ||||||
|       return json.create(EnumSet.of( |       return json.create(EnumSet.of( | ||||||
|           ListChangesOption.CURRENT_REVISION, |           ListChangesOption.CURRENT_REVISION, | ||||||
|           ListChangesOption.CURRENT_COMMIT)) |           ListChangesOption.CURRENT_COMMIT)) | ||||||
|         .format(ids); |         .formatChangeDatas(cds); | ||||||
|     } catch (OrmException | IOException e) { |     } catch (OrmException | IOException e) { | ||||||
|       log.error("Error on getting a ChangeSet", e); |       log.error("Error on getting a ChangeSet", e); | ||||||
|       throw e; |       throw e; | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private List<Change.Id> getForOpenChange(Change c) |   private List<ChangeData> getForOpenChange(Change c) | ||||||
|       throws OrmException, IOException { |       throws OrmException, IOException { | ||||||
|     ChangeSet cs = mergeSuperSet.completeChangeSet(dbProvider.get(), c); |     ChangeSet cs = mergeSuperSet.completeChangeSet(dbProvider.get(), c); | ||||||
|     return cs.ids().asList(); |     return cs.changes().asList(); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private List<Change.Id> getForMergedChange(Change c) |   private List<ChangeData> getForMergedChange(Change c) | ||||||
|       throws OrmException, IOException  { |       throws OrmException, IOException  { | ||||||
|     String subId = c.getSubmissionId(); |     String subId = c.getSubmissionId(); | ||||||
|     List<ChangeData> cds = queryProvider.get().bySubmissionId(subId); |     List<ChangeData> cds = queryProvider.get().bySubmissionId(subId); | ||||||
| @@ -109,14 +109,14 @@ public class SubmittedTogether implements RestReadView<ChangeResource> { | |||||||
|       return Collections.emptyList(); |       return Collections.emptyList(); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     List<Change.Id> ids = new ArrayList<>(cds.size()); |     List<ChangeData> sorted = new ArrayList<>(cds.size()); | ||||||
|     for (PatchSetData psd : sorter.get().sort(cds)) { |     for (PatchSetData psd : sorter.get().sort(cds)) { | ||||||
|       ids.add(psd.data().getId()); |       sorted.add(psd.data()); | ||||||
|     } |     } | ||||||
|     return ids; |     return sorted; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private List<Change.Id> getForAbandonedChange() { |   private List<ChangeData> getForAbandonedChange() { | ||||||
|     return Collections.emptyList(); |     return Collections.emptyList(); | ||||||
|   } |   } | ||||||
| } | } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Dave Borowitz
					Dave Borowitz