Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 337805

Summary: Repo config logic broken when supportingAudits/Branches props missing
Product: [Modeling] EMF Reporter: Caspar D. <caspar_d>
Component: cdo.coreAssignee: Eike Stepper <stepper>
Status: CLOSED FIXED QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: stepper
Version: 4.0Flags: caspar_d: review? (stepper)
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch none

Description Caspar D. CLA 2011-02-22 05:32:18 EST 
The store is created after the repo, yet the repo config logic
assumes it can call on the store to obtain the audit/branching support
from the store if the repo config has no properties to that effect.
This gives a NullPointerException from Repository.java:291 or 301

Either we need to fix the logic; or we need to consider the properties
mandatory. But the code in Repository.setProperties suggests that they're
not.
Comment 1 Eike Stepper CLA 2011-02-22 05:46:03 EST 
(In reply to comment #0)
> The store is created after the repo, 

Usually the store is created *before* the repository is created.

> yet the repo config logic
> assumes it can call on the store to obtain the audit/branching support
> from the store if the repo config has no properties to that effect.
> This gives a NullPointerException from Repository.java:291 or 301

The code assumes that the store is passed into the repo before the properties are injected and that properties are actualley injected at all. This seems indeed strange.

I think I changed this in the context of bug 301512 so that the new delegating mapping strategy can decide early which concrete delegate to create. Any suggestions on how to solve this?
Comment 2 Caspar D. CLA 2011-02-22 22:44:35 EST 
(In reply to comment #1)

> Any suggestions on how to solve this?

Let's remove the automagic smartness and throw an exception if the config
param is missing from the config file.

Patch in a sec.
Comment 3 Caspar D. CLA 2011-02-22 22:48:34 EST 
Created attachment 189572 [details]
Patch
Comment 4 Caspar D. CLA 2011-02-27 23:56:28 EST 
Notwithstanding my patch (which does eliminate the problem), I actually
think the whole approach is wrong here. Requiring the repo to be passed
into the store factory, so that the mappingStrategy can be configured
correctly, is like passing a car into a wheel factory so that some bolt
can be sized correctly. The way I see it, wheel factories shouldn't
require cars to do their job, they should just receive the dimensions of
the wheel...
Comment 5 Eike Stepper CLA 2011-02-28 01:58:16 EST 
I agree. But the repo properties could be passed down. That would eliminate the wrong dependency and still allow a delegating mapping strategy to access the auditing/branching properties ;-)
Comment 6 Eike Stepper CLA 2011-03-02 01:04:14 EST 
Committed revision 7315:
- trunk/plugins/org.eclipse.emf.cdo.server
- trunk/plugins/org.eclipse.emf.cdo.server.db
- trunk/plugins/org.eclipse.emf.cdo.server.db4o
- trunk/plugins/org.eclipse.emf.cdo.server.hibernate
- trunk/plugins/org.eclipse.emf.cdo.server.mongodb
- trunk/plugins/org.eclipse.emf.cdo.server.objectivity
Comment 7 Eike Stepper CLA 2011-03-02 01:05:08 EST 
I removed the IIRepository dependency from:

public interface IStoreFactory
{
  public String getStoreType();
  public IStore createStore(String repositoryName, Map<String, String> repositoryProperties, Element storeConfig);
}
Comment 8 Eike Stepper CLA 2011-03-02 11:47:46 EST 
Committed revision 7319:
- trunk/plugins/org.eclipse.emf.cdo.server
- trunk/plugins/org.eclipse.emf.cdo.tests
Comment 9 Eike Stepper CLA 2011-06-23 03:39:41 EDT 
Available in R20110608-1407