Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change BackEndDataProvider into an interface #8268

Merged
merged 1 commit into from Jan 19, 2017

Conversation

Legioth
Copy link
Member

@Legioth Legioth commented Jan 18, 2017

BackEndDataProvider is now an interface with methods for setting
sorting options based on SortOrder instances.

AbstractBackEndDataProvider stores sorting options, combines them with
the sorting provided in the query and invokes its own abstract fetch and
size methods.

CallbackDataProvider implements a BackEndDataProvider based on two
lambdas.

This is one of many steps towards #8245


This change is Reviewable

@Legioth Legioth force-pushed the fix-abstract_BackEndDataProvider branch from 6212443 to c88cc21 Compare January 18, 2017 11:32
@Legioth Legioth changed the title Make BackEndDataProvider abstract Change BackEndDataProvider into an interface Jan 18, 2017
@caalador
Copy link
Contributor

Reviewed 11 of 11 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@denis-anisimov
Copy link

Reviewed 11 of 11 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java, line 61 at r1 (raw file):

     *            previously set sort orders
     */
    void setSortOrder(SortOrder<String> sortOrder);

It may have default impl.


server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java, line 67 at r1 (raw file):

        return false;
    }
}

new line


server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java, line 35 at r1 (raw file):

 *            data provider filter type
 */
public class CallbackDataProvider<T, F>

I would say CallbackDataProvider is a bad name here.
I have no idea from the name how "callback" is used here.

I'm not sure that I can come up with a better name
But may be DelegateDataProvider ?


Comments from Reviewable

BackEndDataProvider is now an interface with methods for setting
sorting options based on SortOrder instances. 

AbstractBackEndDataProvider stores sorting options, combines them with
the sorting provided in the query and invokes its own abstract fetch and
size methods.

CallbackDataProvider implements a BackEndDataProvider based on two
lambdas.

This is one of many steps towards #8245
@Legioth Legioth force-pushed the fix-abstract_BackEndDataProvider branch from c88cc21 to 0b47c4b Compare January 18, 2017 13:53
@Legioth
Copy link
Member Author

Legioth commented Jan 18, 2017

Review status: 8 of 11 files reviewed at latest revision, 3 unresolved discussions.


server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java, line 61 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

It may have default impl.

Nice catch! Done.


server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java, line 67 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

new line

Done.


server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java, line 35 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

I would say CallbackDataProvider is a bad name here.
I have no idea from the name how "callback" is used here.

I'm not sure that I can come up with a better name
But may be DelegateDataProvider ?

To me, "delegate" is even less specific than what "callback" is.

Two related ideas would be LambdaDataProvider or FunctionDataProvider.


Comments from Reviewable

@denis-anisimov
Copy link

Reviewed 2 of 3 files at r2.
Review status: 10 of 11 files reviewed at latest revision, 1 unresolved discussion.


server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java, line 35 at r1 (raw file):

Previously, Legioth (Leif Åstrand) wrote…

To me, "delegate" is even less specific than what "callback" is.

Two related ideas would be LambdaDataProvider or FunctionDataProvider.

Definitely not Lambda .

Callback wouldn't be so bad in case there wasn't confusion about "consumes a callback" against "it is (or gives) a callback".
I would assume from the name that it is a callback.

I don't know.
Unblocked.


Comments from Reviewable

@denis-anisimov
Copy link

Reviewed 1 of 3 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@denis-anisimov denis-anisimov merged commit dafc831 into master Jan 19, 2017
@denis-anisimov denis-anisimov deleted the fix-abstract_BackEndDataProvider branch January 19, 2017 07:12
pleku pushed a commit that referenced this pull request Feb 9, 2017
* Add getHeaderCellByCaption method to tb-api GridElement (#8248)

* Add getHeaderCellByCaption method to tb-api GridElement

* Make data providers statefull with regards to default sort orders (#8247)

* Make data providers statefull with regards to default sort orders

This is one of many steps towards #8245

* Better instructions in widgetset optimization view in the debug window. (#8261)

* Better instructions in widgetset optimization view in the debug window.

The generated example code is ok, but steps to use them are not obvious.
Added more details to debug window instructions to make it clear what steps are
needed to optimize the widgetset.

Change-Id: Id46632697044223092ef772f98b1fcd69c28d0da

* Simplified javadoc (#8272)

* Simplified javadoc

* Allow setting filters directly in ListDataProvider (#8267)

* Allow setting filters directly in ListDataProvider

This is one of many steps towards #8245

* Change BackEndDataProvider into an interface (#8268)

* Change BackEndDataProvider into an interface

BackEndDataProvider is now an interface with methods for setting
sorting options based on SortOrder instances. 

AbstractBackEndDataProvider stores sorting options, combines them with
the sorting provided in the query and invokes its own abstract fetch and
size methods.

CallbackDataProvider implements a BackEndDataProvider based on two
lambdas.

This is one of many steps towards #8245

* Introduce DateTimeFile and InlineDateTimeField. (#8218)

* Introduce DateTimeFile and InlineDateTimeField.

Fixes #8132

* Correct and provide declarative tests.

* Provide a date converter and UI tests.

* Overload AbstarctComponent.setDescription() with content mode parameter (#8252)

* Overload AbstarctComponent.setDescription() with content mode parameter

Fixes #8185

* Add a data provider wrapper with a configurable filter (#8280)

* Add a data provider wrapper with a configurable filter

This is one of many steps towards #8245

* Updated tutorial to v.8.0.0.beta1 (#8298)

* Add ListDataProvider shorthands for filter conversion (#8279)

Also updates ComboBox.setItems to use these new shorthands

This is one of many steps towards #8245

* Add migration guide to documentation (#8283)

* Migration guide 1st draft

* Restructure and improve migration guide

* Update migrating-to-vaadin8.asciidoc

* Remove outdated filtering concepts (#8289)

* Remove outdated filtering concepts

withFilter is removed since filters are supposed to be set as state in
the data provider instead of by wrapping.

AppendabileFilterDataProvider is removed since the data provider is
supposed to do any appending when updating its own state instead of
through wrapping.

This is one of many steps towards #8245

* Add shorthands to ComboBox for setting a ListDataProvider

* Code cleanup: Move dependency loading logic to a separate class (#8290)

* Fix BeanBinderPropertySet to not deserialize into multiple instances (#8305)

* Fix BeanBinderPropertySet to not deserialize into multiple instances

* Remove custom preloading support and load scripts using async=false (#8291)

When using async='false' for scripts created by scripts
the execution order is guaranteed to be the same as the order the
script tags are created

Fixes #5339, #3631

* Fix absolute layout component sizing on size change (#8269)

* Fix absolute layout component sizing on size change (#8255, #8256, #8257)

Changing from relative to defined size leaves the component positioned wrong,
also changing from defined size to relative size leaves the component in the
wrong position with the wrong size.

Change makes the wrapper rerun setChildWidgetPosition() that adds/removes
necessary css definitions to/from the cssPosition String.

* Clean test sources according to review.

* Use getFirstSelectedItem() instead of getFirstSelected(). (#8308)

* Use getFirstSelectedItem() instead of getFirstSelected().

Fixes vaadin/framework8-issues#596

* Unify container wrapping (#8312)

* Method names unified to withXyzFilter
* withConfigurableFilter moved to DataProvider
* Remove DataProviderWrapper.convert since it does the same as
DataProvider.withConvertedFilter
* Make javadocs slightly more consistent
* Make type parameters consistent

* Make BeanBinderPropertySet exclude Object properties (#8306)

* Make BeanBinderPropertySet exclude Object properties

* Support using multiple @javascript and @Stylesheet annotations on a class (#8293)

* Update data provider documentation to describe the new design (#8317)

* Add static helpers for creating callback data providers (#8314)

Cannot overload the same create method for this purpose since it
conflicts with T... overload. Each case is instead given its own
descriptive but still discoverable method name.

* Serve VAADIN files also from META-INF/resources (#8286) (#8320)

* Serve VAADIN files also from META-INF/resources (#8286)

Fixes #8206

* Allow distinct configurable filter and wrapped filter types (#8322)

* Allow distinct configurable filter and wrapped filter types

This enables implementing the use case that was already described
towards the end of datamodel-providers.asciidoc.

Also swaps the order of the callback type parameters to make all three
match the order of the ConfigurableFilterDataProvider type parameters.

* Use non-Generic SortOrder in Query. (#8239)

* Use non-Generic SortOrder in Query.

Fixes #8215

* Refactor how DataCommunicator passes filters to its DataProvider (#8309)

The immediate benefit of this change is that ComboBox doesn't have to do
any wrapping when a ListDataProvider is set. A secondary benefit is that
a bunch of redundant type parameters and unsafe casts can be removed.

This is one of many steps towards #8245

* Support loading of HTML imports using @HtmlImport (#8301)

Note that not all browsers yet support HTML imports. If a polyfill
is needed to load HTML imports, it must be loaded before HTML Imports
can be loaded. There is no automatic loading of any polyfills.

* Add APIs to inform components of stale objects in DataProvider (#8271)

* Add DataProvider refreshItem for single item update

* Add 'id' concept for DataProviders

This patch also adds a simplified data provider that can replace items
based on their id. This can be used to simulate stale objects from an actual
backend.

* Add refresh logic to Grid SelectionModels

* Remove broken equals and hashCode

* Refresh KeyMapper, clean up some methods

* Fix UI.access in test

* Fix tests and Grid single selection model

* Do clean up before replacing data provider

* Check correct variable for null value

* Fix other selects, add generic tests

* Code style fixes, removed assert

* Merge remote-tracking branch 'origin/master' into 286_refresh_items

* Fix documentation for refreshing an item

* Improve introduction chapter, minor clarifications

* Merge remote-tracking branch 'origin/master' into 287_refresh_items

* Add missing parameters in unit tests

* Show unsupported browser page for FF version less than 45. (#8328)

Fixes vaadin/framework8-issues#569

* Update @HtmlImport javadoc to match implementation (#8331)

* Extract Bean validation logic from Binder to BeanValidationBinder

Fixes #8253

* Fix some faulty javaDoc names

* JspIntegrationTest as a separate module (#8228)

Part of vaadin/framework8-issues#539

* Implement GridSortOrder and SortOrderBuilders (#8338)

* Add convenience constructors for TextField (#597) (#8341)

Add convenience constructors for TextField (#597)

Added TextField convenience constructors similar to the 
ones found in Vaadin framework 7

* Add a default getOptionalValue() to HasValue (#8324)

Fixes #8323

* Allow to disable user selection in the grid model. (#8345)

* Allow to disable user selection in the grid model.

Fixes vaadin/framework8-issues#546

* Introduce empty selection functionality for NativeSelect. (#8336)

* Introduce empty selection functionality for NativeSelect.

Fixes vaadin/framework8-issues#545

* Add regression tests for HasValue.getOptionalValue (#8352)

* Add convenience constructors to new components (#598) (#8351)

Add convenience constructors (#598)

Added convenience constructors to ui components that have
been reimplemented for Vaadin 8

* Fix NPE when grid is sorted by column with null values 
Fixes #8282

* Add reference to Column.setId in javadocs of methods accepting col ids (#8360)

* Increase threshold in server side performance test for stability (#8370)

* Make compatibility aliases for isRequiredIndicatorVisible() (#8372)

* Make compatibility aliases for isRequiredIndicatorVisible()

Fixes #8337

* Fix userOriginated state for CheckBox ValueChangeEvent from the user (#8383)

* Fix userOriginated state for CheckBox ValueChangeEvent from the user

* Refactor editor API to use Binding instead of a component generator (#8368)

Fixes #8366

* Patameterize the ValueChangeListeners for rebuilt fields (#8369) (#8388)

* Patameterize the ValueChangeListeners for rebuilt fields (#8369)

Listeners are now defined with the correct type parameter for
each field type so the event.getValue() is what one would expect.

* Add styling to the unsupported browser page (#570) (#8354)

* Fix documentation examples

* Add test for compiling a widgetset that inherits Vaadin7WidgetSet (#8403)

* Add test for compiling a widgetset that inherits Vaadin7WidgetSet

* Remove unnecessary dependency management block

* Minor style fixes

* Revert widget-set-testutil parent change (#8404)

* Revert widget-set-testutil parent change

* Add a maven plugin for formatting according to Eclipse settings

Running formatter:format will format all files in a submodule using an
embedded Eclipse compiler (based on Neon SR1)
according to the regular formatting settings file.

* Introduce Binder.addFialdValueChangeListener (#8273)

Fixes #8273

* Make Grid add columns based on bean properties (#8392)

* Make Grid add columns based on bean properties

The property set concept used for Binder is slightly generalized and
used by Grid as well to support similar functionality.

Fixes vaadin/framework8-issues#250

* Update release notes for 8.0.0.beta2 (#8402)

* Update release notes for 8.0.0.beta2

* Refactor testing of the framework (#8393)

* Integration tests moved from uitest to separate test modules
* Run TB4 tests with maven profile
* Remove old ant/ivy build scripts from uitest
* Add 'needs-ssh' test category for reconnection test
* Add default values for testing and validation builds
* Add placeholder build.xml with instructions to run Maven

* Replace FontAwesome with Vaadin Icons (#8208)

* Include FontAwesome by default for easier migration
* Fix JSComponentLoadingIndicatorTest

Fixes #7979
Addresses part of #8219

* Require Maven 3.1 or newer for uitest (#8418)

* Make it easy to invoke ServerRpc methods as part of unit tests (#8346)

Executing RPC methods is useful when testing component interaction with
the server. Not only inside the framework but also when creating add-ons
and applications.

* Add helpers for dealing with columns based on its id (#8411)

* Add helpers for dealing with columns based on its id

setColumns(Column...)  is removed since it's not possible to re-add a
removed column instance.

Fixes #8361

* Handle system properties for failsafe plugin (#8414)

* Handle system properties for failsafe plugin

* Grid tests migration. Next round. (#8412)

* Grid tests migration. Next round.

Fixes #8373

* Add scrollTo methods to Grid (#8203) (#8410)

* Add scroll methods to serverside grid (#8203)

Added scrollToTop(), scrollToEnd() and scrollTo(int row)

* Fix scrolling to view of opened details (#8203)

Removed dependency for DetailsManagerConnector from GridConnector.
GridConnector now handles one off listeners.

* Rename detailsRefresh to better show that it's one-off.
Add missing copyright header.

* Add header required by docs site (#8425)

* Add header required by docs site

* Typo fix; screenshots update.

* Add default getEmptyValue for MultiSelect interface (#8420) (#8421)

* Add deafult getEmptyValue for MultiSelect interface (#8420)

Added an default implementation for getEmptyValue to the
MultiSelect interface.

* Add Grid editor events. (#8365)

* Migrate v7 Grid tests. Next round.

Fixes #8419

* Add Grid editor events.

Fixes #8202.

* Run tests on Firefox 45 (#8428)

* Run tests on Firefox 45

* Fix some broken tests and ComboBoxElement (#8430)

* Fix absolute layout component resize test (#8429)

Fix test checking for left:auto by testing the actual wrapper width
that should change.

* Add selection methods to grid that delegate to its selection model (#8315)

* Add selection methods to grid that delegate to the selection model

* Merge remote-tracking branch 'github/master' into 577-alt

* Remove getFirstSelected, isSelected

* Add isValid to Binder (#8274)

* Add isValid to Binder

isValid enables you to check the current validation
status of a given Binder, without firing events or invoking
handlers.

This patch also clarifies the javadocs of StatusChangeEvents'
hasValidationErrors.

* Throw in Binder.isValid if no bean is set and bean validators exist

* Add test cases to BinderTest

* Update ValoThemeTest screenshots (#8431)

* Fix exception on remove column. (#8432)

Schedule column ordering so that possible onUnregister methods get run first.

* Add Firefox 45 screenshots (#8438)

* Fix JBoss 6 integration tests

* Disable test method on which build got stuck. (#8445)

* Add Firefox 45 workaround in ComboBoxElement (#8449)

* Add screenshots and workaround for Firefox 45 (#8450)

* Fix problem with re-opening the popup (#8446) (#8451)

* Fix problem with re-opening the popup (#8446)

Fixed problem that disables opening of popup
after closing it by clicking the datefield-button.

* Fix issue #8446 in compatibility version PopupDateField also.

* Add missing ScreenshotRule to Spring integration test (#8455)

* Make one-arg addColumn use natural sort for mutually comparable values (#8458)

Fixes #8371
Fixes #8128

* Use thread-safe collections for VaadinService listeners (#7037)

Fixes #7250

* Mention removed ThreadLocal.setInheritable in release notes (#8466)

Fixes vaadin/framework8-issues#594

* Added missing parametrisation onto the column field of the GridContextClickEvent. (#8473)

* Added missing parametrisation onto the column field of the GridContextClickEvent.

* Fix toggling of WeekNumbers for DateTimeField. (#8468)

Fixes  #8405

* Add Grid.addColumn(String, Renderer) (#8470)

* Add Grid.addColumn(String, Renderer)

* Throw if there are no automatically bound fields via bindInstanceFields. (#8481)

* Throw if there are no automatically bound fields via bindInstanceFields.

Fixes #8362

* Fix keyboard navigation for ListSelect (#8503)

Fixes vaadin/framework8-issues#463

* Fix profiles in vaadin-root pom.xml (#8484)

* Add javadoc explanation about how to create serializable comparators (#8505)

Also removes the confusing SerializableComparator.asInstance method that
was mainly introduced for internal use.

Fixes #8367

* Provide a way to configure bean binder to auto set required fields (#8460)

@NotNull, @SiZe(min>1), @notempty annotations are handled via default
configuration which marks fields as required.

Fixes #8382

* Document BeanValidationBinder and RequiredFieldConfigurator. (#8504)

This patch also provides a documentation for #8382.

Fixes #8390

* Fix removing and adding a column in grid (#8437)

* Added column should also be added to footer (#590)

When adding (readding) a column the column should also be
added to footer as it will be removed and iterator will request footer for it.

* Add test for re-add column (#590)

* To lowerCase using Locale

* Fixed compilation problem.
Too little sleep for a friday evening...

* Provide a way to set styles for Header/Footer Cells and Rows in a Grid (#8499)

Fixes #8422

* Extract data provider callbacks to named and documented interfaces (#8508)

Fixes #8488

* Provide a convenience method to get items set via setItems(). 

Fixes #8357

* Allow configuring how string to number converters handle empty strings (#8506)

* Clarify javadocs for query values not used for counting (#8509)

Fixes #8489

* Add addComponentsAndExpand to horizontal and  vertical layout (#8480)

* HTML5 Drag and Drop Support (#8264)

* Add DragSource Extension (#8169)

* Add DropTarget Extension (#8170)

* Add DragStart Event to DragSource Extension (#8171)

* Make DataTransfer.dropEffect configurable (#8174)

* Make DragSource.dataTransfer data configurable (#8172)

* Add server-side Event for drop (#8177)

* Added license headers

* Extract handler methods, move DropEvent and DropListener to new file, move enums to top

* Replaced LinkedHashMap with Map and added List to preserve order of data

* Add API for adding a JS acceptance criteria for dragover and drop (#8178, #8179)

* Make DragSource Extension extendable (#8175)

* Make DropTarget Extension extendable (#8176)

* Added javadoc to protected methods

* Moved EffectAllowed to shared so that it could be used in shared state directly

* Moved DropEffect to separate file, some review fixes and javadoc

* Added list to DropTargetRpc to preserve order of data

* Remove event listeners on unregister

* Changed method names set/getData() to more descriptive set/getTransferData()

* Add server side dragStart event (#8171)

* Add style to prevent text selection to allow drag

* Remove target indicator style on drop

* Add client side dragend event listener for drag source

* Add server side dragend listener.
Attach client side listener only when server side listener added.

* Add drag source information to server side dragstart and dragend events.

* Fixed some issues addressed in review

* Trigger server side dragstart only when there is a listener attached

* Criteria script can be set as null to clear

* Use Js Interop instead of JSNI for handling event listeners

* Use elemental package instead of Js Interop for handling event listeners

* Add missing javadoc for public methods

* Add default value "uninitialized" to effectAllowed parameter

* Simple test UI for HTML5 DnD functionality (#8395)

* Add javadoc and other minor changes
@tsuoanttila tsuoanttila added this to the Legacy milestone Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants