This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 54451

Summary:	SearchHistory API change
Product:	utilities	Reporter:	Martin Roskanin <mroskanin>
Component:	Search	Assignee:	Martin Roskanin <mroskanin>
Status:	RESOLVED FIXED
Severity:	blocker	CC:	apireviews, issues
Priority:	P1	Keywords:	API_REVIEW_FAST
Version:	4.x
Hardware:	All
OS:	All
Issue Type:	TASK	Exception Reporter:
Attachments:	API change diffs updated diff

Description Martin Roskanin 2005-02-03 10:20:09 UTC

API change is needed because of the fix of issue
#54028
It is necessary to fire further
PropertyChangeEvent to fix it. (Details in the
issue #54028)It is event ADD_TO_HISTORY, that will
be fired after adding new SearchPattern to
history. Old value of event would be null, new
value would be added pattern.
Also pattern checking before adding to history is
a subject of the API change.

If pattern is null, its search expression is null
or empty, or there is already the same pattern
item in the top of the history list, the pattern
will not be added (event will not be fired)

I will create some test(s) and attach it together
with API change diffs.

Comment 1 Martin Roskanin 2005-02-03 10:43:17 UTC

Created attachment 20171 [details]
API change diffs

Comment 2 Jaroslav Tulach 2005-02-03 12:09:44 UTC

1. diff -u is much easier to read
2. should not you increase the version number in manifest.mf to 3.6?
3. in tests it is useful to check that some things do not happen. Like:
>     public void testAddToSearchHistoryListener() throws Exception{
>         final boolean fired[] = new boolean[2];
>         PropertyChangeListener pcl = new PropertyChangeListener(){
>                 public void propertyChange(PropertyChangeEvent evt){
>                     if (evt!=null &&
SearchHistory.ADD_TO_HISTORY.equals(evt.getPropertyName())){
>                         fired[0] = true;
>                     } else {
>                         fired[1] = true;
>                 }
>         };
>         SearchHistory.getDefault().addPropertyChangeListener(pcl);
>        
SearchHistory.getDefault().add(SearchPattern.create("searchtext",true,true,false));
>         SearchHistory.getDefault().removePropertyChangeListener(pcl);
>         assertTrue(fired[0]);
>         assertFalse("Only the expected change is fired", fired[1]);
>     }

Otherwise, this change seems to have all I was asking for.

Comment 3 Martin Roskanin 2005-02-03 13:33:29 UTC

Thanks for the review and test tip.
Ad 1: I am attaching diff -u with recommended test changes. 
ad 2: Should a version be increased if the API change is compatible?

Comment 4 Martin Roskanin 2005-02-03 13:34:22 UTC

Created attachment 20174 [details]
updated diff

Comment 5 mslama 2005-02-03 13:42:43 UTC

Yes version is always increased. And you did it already in apichanges.xml.

Comment 6 Martin Roskanin 2005-02-03 13:56:19 UTC

OK, I have increased a version to 3.6 in manifest.mf

Comment 7 Martin Roskanin 2005-02-14 09:19:14 UTC

I am about to commit the changes within 24 hours

Comment 8 Martin Roskanin 2005-02-15 09:21:33 UTC

fixed in [maintrunk]

/cvs/openidex/manifest.mf,v  <--  manifest.mf
new revision: 1.34; previous revision: 1.33

/cvs/openidex/api/apichanges.xml,v  <--  apichanges.xml
new revision: 1.4; previous revision: 1.3

/cvs/openidex/src/org/openidex/search/SearchHistory.java,v  <-- 
SearchHistory.java
new revision: 1.4; previous revision: 1.3

/cvs/openidex/test/unit/src/org/openidex/search/SearchHistoryTest.java,v
 <--  SearchHistoryTest.java
new revision: 1.2; previous revision: 1.1