Hi, Anyone interested to work on this ?
I like the improvement. But I also am afraid of the API break! If there is a way to use an adapter for the old impls then it will be great! I'd be glad to help but I don't have the complete picture to do it by myself. Martin Grigorov Wicket Training and Consulting On Thu, May 15, 2014 at 11:24 AM, Martijn Dashorst < [email protected]> wrote: > > > > On 7 mei 2014, at 11:20, Leszek Gawron <[email protected]> wrote: > > > > Hibernate Session is actually a little bit different - Hibernate devs > > made it lightweight on purpose. It does not fetch db connection even > > if created preemptively. > > > > Let's call the "page" a DataPage. In my own half-baked solutions I > > usually used a very skinny DataPage implementation with not much more > > than iterator() and size(). > > Some time ago I started using Spring DATA Jpa. Their page interface is > > very fat because it is backed up with a default PageImpl. > > > > First I was very hesitant about this solution but actually this > > approach encapsulates a lot of calculations (current page, available > > page count etc) in single class and makes all classes using it a > > little bit simpler. > > > > Have a look at: > > > > > https://github.com/spring-projects/spring-data-commons/tree/master/src/main/java/org/springframework/data/domain > > > > in particular: Page.java, PageImpl.java, Pageable.java > > > > It's probably a overkill for wicket needs but we may use some of those > > ideas. We could start simple and see how much functionality suits us > > in DataPageImpl. > > > > > > Currently we have this functionality dispersed over some classes that > > shouldn't even perform those calculations, like AbstractPageableView: > > > > /** > > * @see > org.apache.wicket.markup.html.navigation.paging.IPageable#getPageCount() > > */ > > @Override > > public long getPageCount() > > { > > long total = getRowCount(); > > long itemsPerPage = getItemsPerPage(); > > long count = total / itemsPerPage; > > > > if (itemsPerPage * count < total) > > { > > count++; > > } > > > > return count; > > > > } > > > > /** > > * @return the index of the first visible item in the view > > */ > > public long getFirstItemOffset() > > { > > return getCurrentPage() * getItemsPerPage(); > > } > > > > or PagingNavigationIncrementLink: > > > > /** > > * Determines the next page number for the pageable component. > > * > > * @return the new page number > > */ > > public final long getPageNumber() > > { > > // Determine the page number based on the current > > // PageableListView page and the increment > > long idx = pageable.getCurrentPage() + increment; > > > > // make sure the index lies between 0 and the last page > > return Math.max(0, Math.min(pageable.getPageCount() - 1, idx)); > > } > > > > /** > > * @return True if it is referring to the first page of the > > underlying PageableListView. > > */ > > public boolean isFirst() > > { > > return pageable.getCurrentPage() <= 0; > > } > > > > /** > > * @return True if it is referring to the last page of the > > underlying PageableListView. > > */ > > public boolean isLast() > > { > > return pageable.getCurrentPage() >= (pageable.getPageCount() - 1); > > } > > > > > > If you are willing to accept a pull request for this functionality I > > could start some coding. I wanted to get your acceptance first as this > > is not an obvious change. I am quite sure we can keep all (or at least > > most) current public API intact. > > > > If all public API can be kept can I base my coding on 6.15? > > > > lg > > > > > >> On Wed, May 7, 2014 at 12:05 AM, tetsuo <[email protected]> > wrote: > >> Yeah, you're right. By reflex, I was thinking Hibernate Session (with > open > >> session in view), not JDBC Connection, since I don't work directly with > it > >> very often. And even if it was Hibernate, and the session was still > open, > >> the connection would also be closed anyway. > >> > >> What methods would the Page interface (with another name, since we > already > >> have a Page class) have? iterator() and size()? > >> > >> > >> > >> “The truth is rarely pure and never simple.” ― Oscar Wilde > >> > >> > >>> On Tue, May 6, 2014 at 5:41 PM, Leszek Gawron <[email protected]> > wrote: > >>> > >>> I replied from wrong email, please excuse me if this reaches the list > >>> twice. > >>> > >>>> On Tue, May 6, 2014 at 8:06 PM, tetsuo <[email protected]> > wrote: > >>>> Since it's the same request, isn't (or couldn't be) the db connection > the > >>>> same in size() and iterator() calls? If you created the temporary > table > >>> in > >>>> one, isn't it still available to the other (so you could avoid > recreating > >>>> it)? > >>> Well, the connection is being managed by spring transactional resource > >>> management classes. A transaction is demarcated with > >>> @Transactional(readOnly=true) on a particular method. All > >>> transactional resources are bound to thread during transaction > >>> execution and released when transaction finishes. One transactional > >>> resource is of course a connection. I have extended this framework to > >>> register temporary table names which are being automatically deleted > >>> on transaction finish. > >>> > >>> Each time you invoke a transactional method you perform following > steps: > >>> - acquire a connection from the pool > >>> - bind connection to thread making all subsequent queries use same > >>> connection > >>> - start transaction > >>> - do some work > >>> - finish transaction (of course for read only there is nothing to be > done) > >>> - release transactional resources > >>> - unbind connection from thread > >>> - release connection to the pool. > >>> > >>> using spring framework without spring managed transactions means you > >>> end up using different connection for EVERY sql query. > >>> > >>> My IDataProviders usually wrap spring bean with > >>> @Transactional(readOnly=true) methods. > >>> > >>> Unfortunatelly two method calls: size() and iterator() mean two > >>> different transaction and two different connections. I am unable to > >>> wrap any single point of entry with @Transactional - that probably > >>> would have to be some generic Wicket method call. I cannot bind a > >>> connection to current request thread preemptively because that would > >>> mean a huge resource hog - I would have to obtain a connection from > >>> pool for every request being made. > >>> > >>> Temporary tables have connection scope so not deleting the table on > >>> connection release means polluting the database server with lots of > >>> temporary tables as long as connection stays in pool (which could be > >>> indefinite). > >>> The temporary tables would be probably stale all the time - any > >>> ongoing transaction may probably alter what the temporary table should > >>> contain. > >>> > >>> Anyway any approach still feels like a hack and not resolving the > >>> problem at source. > >>> > >>> > >>> > >>> A single point of entry in IDataProvider: > >>> - allows the user to manage resources more efficiently. > >>> - gives user the option to resign from providing size() information - > >>> do you really go from 1st to 15th page that often or do you usually > >>> click "next results"? > >>> - gives user the option to provide "has more results" information > >>> without providing actual dataset size. > >>> > >>> Having a single point of entry IDataSource means also you can do: > >>> > >>> new IDataProviderSingleEntry<T( oldDataProvider ) { > >>> public Page<T> getPage( long first, long count ) { > >>> return new PageImpl<T>( > >>> oldDataProvider.interator( first, count ), > oldDataProvider.size() > >>> ); > >>> } > >>> } > >>> > >>> This trivial decorator means IDataProviderSingleEntry and > >>> IDataProvider may even coexist in wicket-core and wicket-extensions. > >>> Some variation of this approach is actually used by inmethod-grid. > >>> Maybe we could unify the way all pageables access resources. > >>> > >>> > >>> -- > >>> Leszek Gawron > >>> > > > > > > > > -- > > Leszek Gawron http://www.mobilebox.pl/krs.html > > CTO at MobileBox S.A. > > >
