Skip to content

Commit 1007ea7

Browse files
bbimberbbimber
committed
Refactor laboratory.workbooks and container listener code to be more efficient
1 parent c786b6b commit 1007ea7

File tree

6 files changed

+259
-204
lines changed

6 files changed

+259
-204
lines changed

‎laboratory/src/org/labkey/laboratory/LaboratoryContainerListener.java‎

Lines changed: 89 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,6 @@ public void containerCreated(Container c, User user)
6868
{
6969
_log.warn("Unable to populate default values for laboratory module", e);
7070
}
71-
catch (BatchValidationException e)
72-
{
73-
//ignore, since this may just indicate the table already has these values
74-
}
7571
}
7672
}
7773

@@ -112,69 +108,11 @@ public void propertyChange(PropertyChangeEvent evt)
112108

113109
if (evt.getPropertyName().equals(ContainerManager.Property.Name.name()))
114110
{
115-
if (evt instanceof ContainerManager.ContainerPropertyChangeEvent)
116-
{
117-
ContainerManager.ContainerPropertyChangeEvent ce = (ContainerManager.ContainerPropertyChangeEvent) evt;
118-
if (ce.container.isWorkbook())
119-
{
120-
TableInfo ti = LaboratorySchema.getInstance().getTable(LaboratorySchema.TABLE_WORKBOOKS);
121-
TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString(LaboratoryWorkbooksTable.WORKBOOK_COL), ce.container.getId()), null);
122-
if (ts.exists())
123-
{
124-
try
125-
{
126-
Integer rowId = Integer.parseInt(ce.container.getName());
127-
WorkbookModel w = ts.getObject(ce.container.getId(), WorkbookModel.class);
128-
w.setWorkbookId(rowId);
129-
Table.update(ce.user, ti, w, ce.container.getId());
130-
}
131-
catch (NumberFormatException e)
132-
{
133-
134-
}
135-
}
136-
}
137-
}
111+
updateWorkbookTableOnNameChange(evt);
138112
}
139113
else if (evt.getPropertyName().equals(ContainerManager.Property.Policy.name()))
140114
{
141-
if (evt instanceof ContainerManager.ContainerPropertyChangeEvent)
142-
{
143-
ContainerManager.ContainerPropertyChangeEvent ce = (ContainerManager.ContainerPropertyChangeEvent)evt;
144-
145-
User u = ce.user;
146-
if (u == null && HttpView.hasCurrentView())
147-
u = HttpView.currentView().getViewContext().getUser();
148-
149-
if (u == null || !ce.container.hasPermission(u, InsertPermission.class))
150-
return;
151-
152-
if (ce.container.getActiveModules().contains(ModuleLoader.getInstance().getModule(LaboratoryModule.class)))
153-
{
154-
try
155-
{
156-
LaboratoryManager.get().initWorkbooksForContainer(u, ce.container);
157-
}
158-
catch (Exception e)
159-
{
160-
_log.error("Unable to update laboratory workbooks table", e);
161-
}
162-
163-
//attempt to populate default values on load
164-
try
165-
{
166-
LaboratoryManager.get().populateDefaultData(u, ce.container, null);
167-
}
168-
catch (IllegalArgumentException e)
169-
{
170-
_log.error("Unable to populate defaults in laboratory module tables", e);
171-
}
172-
catch (BatchValidationException e)
173-
{
174-
//ignore, since this may just indicate the table already has these values
175-
}
176-
}
177-
}
115+
possiblyInitializeOnActiveModuleChange(evt);
178116
}
179117
}
180118

@@ -194,4 +132,91 @@ protected void purgeTable(UserSchema userSchema, TableInfo table, Container c)
194132
super.purgeTable(userSchema, table, c);
195133
}
196134
}
135+
136+
/**
137+
* The container name field stores the workbookId as a string. If that name changes (and this should no longer be permitted
138+
* for the most part in LK, we need to update this table
139+
*/
140+
private void updateWorkbookTableOnNameChange(PropertyChangeEvent evt)
141+
{
142+
if (!(evt instanceof ContainerManager.ContainerPropertyChangeEvent))
143+
{
144+
return;
145+
}
146+
147+
ContainerManager.ContainerPropertyChangeEvent ce = (ContainerManager.ContainerPropertyChangeEvent) evt;
148+
if (!ce.container.isWorkbook())
149+
{
150+
return;
151+
}
152+
153+
TableInfo ti = LaboratorySchema.getInstance().getTable(LaboratorySchema.TABLE_WORKBOOKS);
154+
TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString(LaboratoryWorkbooksTable.WORKBOOK_COL), ce.container.getId()), null);
155+
if (ts.exists())
156+
{
157+
try
158+
{
159+
Integer workbookId = Integer.parseInt(ce.container.getName());
160+
WorkbookModel w = ts.getObject(ce.container.getId(), WorkbookModel.class);
161+
w.setWorkbookId(workbookId);
162+
Table.update(ce.user, ti, w, ce.container.getId());
163+
}
164+
catch (NumberFormatException e)
165+
{
166+
_log.error("Non-numeric workbook name: " + ce.container.getName() + " for: " + ce.container.getEntityId());
167+
}
168+
}
169+
}
170+
171+
/**
172+
* The intent of this is to initialize the laboratory folder if the set of active modules
173+
* changes to include Laboratory. This should only occur on the parent folder, not individual workbooks.
174+
*/
175+
private void possiblyInitializeOnActiveModuleChange(PropertyChangeEvent evt)
176+
{
177+
if (!(evt instanceof ContainerManager.ContainerPropertyChangeEvent))
178+
{
179+
return;
180+
}
181+
182+
ContainerManager.ContainerPropertyChangeEvent ce = (ContainerManager.ContainerPropertyChangeEvent)evt;
183+
184+
//Only make these changes from the parent container for performance reasons
185+
if (ce.container.isWorkbook())
186+
{
187+
return;
188+
}
189+
190+
User u = ce.user;
191+
if (u == null && HttpView.hasCurrentView())
192+
{
193+
u = HttpView.currentView().getViewContext().getUser();
194+
}
195+
196+
if (u == null || !ce.container.hasPermission(u, InsertPermission.class))
197+
{
198+
return;
199+
}
200+
201+
if (ce.container.getActiveModules().contains(ModuleLoader.getInstance().getModule(LaboratoryModule.class)))
202+
{
203+
try
204+
{
205+
LaboratoryManager.get().recursivelyInitWorkbooksForContainer(u, ce.container);
206+
}
207+
catch (Exception e)
208+
{
209+
_log.error("Unable to update laboratory workbooks table", e);
210+
}
211+
212+
try
213+
{
214+
LaboratoryManager.get().populateDefaultData(u, ce.container, null);
215+
}
216+
catch (Exception e)
217+
{
218+
_log.error("Unable to populate defaults in laboratory module tables", e);
219+
}
220+
}
221+
}
197222
}

‎laboratory/src/org/labkey/laboratory/LaboratoryController.java‎

Lines changed: 4 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ public boolean handlePost(Object form, BindException errors) throws Exception
459459
{
460460
try
461461
{
462-
LaboratoryManager.get().initWorkbooksForContainer(getUser(), getContainer());
462+
LaboratoryManager.get().recursivelyInitWorkbooksForContainer(getUser(), getContainer());
463463

464464
return true;
465465
}
@@ -725,7 +725,7 @@ public ApiResponse execute(UpdateWorkbookForm form, BindException errors) throws
725725
return null;
726726
}
727727

728-
WorkbookModel model = LaboratoryManager.get().getWorkbookModel(getContainer());
728+
WorkbookModel model = LaboratoryManager.get().getWorkbookModel(getContainer(), true);
729729
if (model == null)
730730
{
731731
errors.reject(ERROR_MSG, "Unable to find workbook record for this folder");
@@ -737,11 +737,11 @@ public ApiResponse execute(UpdateWorkbookForm form, BindException errors) throws
737737
model.setResults(form.getResults());
738738
model.setDescription(form.getDescription(), getUser());
739739

740-
LaboratoryManager.get().updateWorkbook(getUser(), model);
740+
LaboratoryManager.get().createOrUpdateWorkbook(getUser(), model);
741741

742742
if (form.isForceTagUpdate() || form.getTags() != null)
743743
{
744-
LaboratoryManager.get().updateWorkbookTags(getUser(), getContainer(), (form.getTags() == null ? (Collection)Collections.emptyList() : Arrays.asList(form.getTags())));
744+
LaboratoryManager.get().updateWorkbookTags(getUser(), getContainer(), (form.getTags() == null ? Collections.emptyList() : Arrays.asList(form.getTags())));
745745
}
746746

747747
results.put("success", true);
@@ -2256,73 +2256,6 @@ public void setProtocol(Integer protocol)
22562256
}
22572257
}
22582258

2259-
@RequiresPermission(AdminPermission.class)
2260-
public class MigrateWorkbooksAction extends MutatingApiAction<MigrateWorkbooksForm>
2261-
{
2262-
public ApiResponse execute(MigrateWorkbooksForm form, BindException errors) throws Exception
2263-
{
2264-
if (getContainer().isWorkbook())
2265-
{
2266-
errors.reject(ERROR_MSG, "Cannot be run on workbooks");
2267-
return null;
2268-
}
2269-
2270-
//find current workbook #
2271-
2272-
2273-
//find all workbooks where workbookId doesnt match LK ID
2274-
TreeMap<Integer, Container> toFix = new TreeMap<>();
2275-
for (Container c : ContainerManager.getChildren(getContainer()))
2276-
{
2277-
if (c.isWorkbook())
2278-
{
2279-
WorkbookModel w = LaboratoryManager.get().getWorkbookModel(c);
2280-
if (w != null)
2281-
{
2282-
_log.warn("workbook model not found for: " + c.getName());
2283-
}
2284-
else if (!c.getName().equals(w.getWorkbookId().toString()))
2285-
{
2286-
toFix.put(w.getWorkbookId(), c);
2287-
}
2288-
}
2289-
}
2290-
2291-
_log.info("workbooks to migrate: " + toFix.size());
2292-
Set<Integer> list = form.getReverseOrder() == true ? toFix.keySet() : toFix.descendingKeySet();
2293-
for (Integer id : list)
2294-
{
2295-
Container wb = toFix.get(id);
2296-
Container target = ContainerManager.getChild(wb.getParent(), id.toString());
2297-
if (target != null)
2298-
{
2299-
_log.warn("target workbook exists, skipping: " + id);
2300-
}
2301-
else
2302-
{
2303-
ContainerManager.rename(wb, getUser(), id.toString());
2304-
}
2305-
}
2306-
2307-
return new ApiSimpleResponse("success", true);
2308-
}
2309-
}
2310-
2311-
public static class MigrateWorkbooksForm
2312-
{
2313-
private Boolean reverseOrder = false;
2314-
2315-
public Boolean getReverseOrder()
2316-
{
2317-
return reverseOrder;
2318-
}
2319-
2320-
public void setReverseOrder(Boolean reverseOrder)
2321-
{
2322-
this.reverseOrder = reverseOrder;
2323-
}
2324-
}
2325-
23262259
@RequiresPermission(ReadPermission.class)
23272260
public class DataBrowserAction extends SimpleViewAction<Object>
23282261
{

0 commit comments

Comments
 (0)