Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 311093 - Cannot type in the Primary Key Generation 'Generator name' combo
Summary: Cannot type in the Primary Key Generation 'Generator name' combo
Status: VERIFIED FIXED
Alias: None
Product: Dali JPA Tools
Classification: WebTools
Component: General (show other bugs)
Version: 2.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 2.3 RC1   Edit
Assignee: Karen Butzke CLA
QA Contact:
URL:
Whiteboard: PMC_approved
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-29 15:00 EDT by Karen Butzke CLA
Modified: 2010-07-14 13:17 EDT (History)
3 users (show)

See Also:
neil.hauge: pmc_approved? (david_williams)
neil.hauge: pmc_approved? (raghunathan.srinivasan)
neil.hauge: pmc_approved? (naci.dai)
deboer: pmc_approved+
neil.hauge: pmc_approved? (neil.hauge)
neil.hauge: pmc_approved? (kaloyan)


Attachments
proposed patch against head (13.82 KB, patch)
2010-05-07 17:33 EDT, Karen Butzke CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Karen Butzke CLA 2010-04-29 15:00:28 EDT
I am attempting to type in the ID mapping Primary Key Generation 'Generic name' combo. Anything I type in gets automatically deleted.
Comment 1 Neil Hauge CLA 2010-04-29 15:12:39 EDT
This is pretty nasty.  Should fix for RC1.
Comment 2 Karen Butzke CLA 2010-05-07 17:33:59 EDT
Created attachment 167586 [details]
proposed patch against head

This patch must be applied along with the patch on bug 311112 to fully fix this problem.  This patch is isolated to the GeneratedValueComposite and switches that combo to use our ComboModelAdapter instead of handling the combo directly.
Comment 3 Brian Vosburgh CLA 2010-05-08 01:50:46 EDT
So, I spent more than 30 seconds thinking about the persistence unit generators
(and queries) and came up with another idea you might try. Put the following lines of code at the appropriate places at the top and bottom of
AbstractPersistenceUnit.update(...):

        ArrayList<Generator> oldGenerators = new ArrayList<Generator>(this.generators);
        this.generators.clear();
        ArrayList<Query> oldQueries = new ArrayList<Query>(this.queries);
        this.queries.clear();

        ...

        // See comment at top of method.
        // These calls are "backwards" compared to the usual practice -
        // 'generators' and 'queries' are already updated, we are
        // "faking" changes to trigger the appropriate events here.
        this.synchronizeList(this.generators, oldGenerators, GENERATORS_LIST);
        this.synchronizeList(this.queries, oldQueries, QUERIES_LIST);

This might allow you to get rid of the CompositeLVM hack in GeneratedValueComposite?

[Of course, I wait until I've mucked around with CompositeLVM....
Oh well, it's better now. :-) ]
Comment 4 Karen Butzke CLA 2010-05-10 11:05:12 EDT
Ok, with this change the UI ends up displaying toStrings for the generators.  How this ever worked before I'm not completely sure, something to do with actually rebuilding the list every time.  This code is wrong since it doesn't the generator name property change events, I'm going to work on cleaning this up.

protected ListValueModel<String> buildGeneratorNameListHolder() {
    return new ListAspectAdapter<PersistenceUnit, String>(
			buildPersistenceUnitHolder(),
			PersistenceUnit.GENERATORS_LIST)
    {
        @Override
	protected ListIterator<String> listIterator_() {
	    return CollectionTools.listIterator(ArrayTools.sort(this.subject.uniqueGeneratorNames()));
	}
    };
}
Comment 5 Karen Butzke CLA 2010-05-10 14:20:08 EDT
After some attempts at implementing Brian's suggestion in comment 3 we have decided that the potential for more bugs is too great to make that change right now.  I would like to continue ahead with the attached patch which fixes the problems even if not the perfect solution.  I will enter another bug to fix this more correctly in a future release.
Comment 6 Neil Hauge CLA 2010-05-10 17:04:38 EDT
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug"
(requested by an adopter) please document it as such. 

Unable to type into an input field in the UI, contents get deleted after typing.

* Is there a work-around? If so, why do you believe the work-around is
insufficient? 

Only workaround is to edit the source directly.

* How has the fix been tested? Is there a test case attached to the bugzilla
record? Has a JUnit Test been added? 

Manually tested.

* Give a brief technical overview. Who has reviewed this fix? 

See comment 2.  I have reviewed the fix.

* What is the risk associated with this fix? 

Low risk, isolated fix.  Higher risk solution was deferred.
Comment 7 Karen Butzke CLA 2010-05-10 17:49:31 EDT
checked in for RC1
Comment 8 Paul Fullbright CLA 2010-05-18 14:38:15 EDT
entered bug 313420, but verified that you can type in the combo in RC1
Comment 9 Karen Butzke CLA 2010-07-14 13:17:28 EDT
(In reply to comment #3)
Brian's comment here oversimplifies things. I have attempted to change things to work this way without any luck.

I tested with multiple generators and renamed one of them. The problem is that the UI gets notification if a name of a generator has changed and at that point the underlying model list of generators has been cleared and could be partially rebuilt as the update is currently progressing. The ItemAspectListValueModelAdapter fires a listChanged event when an aspect is changed, the list is not yet complete. Then no other change notification is fired because the list itself hasn't actually changed, just the name of an item. This causes the UI list to be too short and later leads to IndexOutOfBoundsExceptions.