Skip to content

Addition of AvroFeatureAdapter#558

Merged
jwomeara merged 1 commit intomasterfrom
GEOWAVE-380
Dec 3, 2015
Merged

Addition of AvroFeatureAdapter#558
jwomeara merged 1 commit intomasterfrom
GEOWAVE-380

Conversation

@mrdahlb
Copy link
Copy Markdown
Contributor

@mrdahlb mrdahlb commented Dec 2, 2015

This new adapter is intented to serialize all the attributes of a SimpleFeature as a single object. This is meant as an optimization when visibility is the same for all attributes of the feature.
Included is the avro schema to create the generated classes from chris bennights Import/Export project, these are necessary for avro serializing/deserializing the SimpleFeature.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to overload this method? I don't think we need the AvroAdaptorProxyFieldLevelVisibilityHandler either. The only difference between AvroAdaptorProxyFieldLevelVisibilityHandler and AdaptorProxyFieldLevelVisibilityHandler is the type of the data adapter. However, I don't think we need to be specific about the adapter type in the Avro visibility handler because the AvroFeatureDataAdapter is a FeatureDataAdapter. In short, I think we can reuse the AdaptorProxyFieldLevelVisibilityHandler in this method, but if we do that, the method remains unchanged from the base implementation, so I don't think we need to overload this method at all. Hopefully that makes sense. If not, let me know.

Also, this is not directed specifically at you as the naming convention has been in place for some time now, but bonus points to you if you add a commit which renames all instances of "adaptor" to "adapter" throughout the project, d'oh! :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, I created this before extending FeatureDataAdapter. I will remove the override and the unnecessary avro handler class.

@mrdahlb mrdahlb force-pushed the GEOWAVE-380 branch 2 times, most recently from 50d2038 to 23343ea Compare December 2, 2015 20:46
…impleFeature as a single object. Included is the avro schema to create the generated types from chris bennights Import/Export project, these are necessary for serializing/deserializing the SimpleFeature
@jwomeara
Copy link
Copy Markdown
Contributor

jwomeara commented Dec 3, 2015

Looks good to me.

jwomeara added a commit that referenced this pull request Dec 3, 2015
Addition of AvroFeatureAdapter
@jwomeara jwomeara merged commit 967db32 into master Dec 3, 2015
@jwomeara jwomeara deleted the GEOWAVE-380 branch December 3, 2015 15:32
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.

2 participants