Skip to content

No option to exclude class name field from generated queries [DATACOUCH-179] #493

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

Closed
spring-projects-issues opened this issue Dec 11, 2015 · 10 comments
Assignees
Labels
in: core Issues in core support in: repository Repositories abstraction status: duplicate A duplicate of another issue type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link

Abhijit Sarkar opened DATACOUCH-179 and commented

Working with the sample bucket travel-sample, I created this method in a CouchbaseRepository:
Airport findByIcao(String airportCode);

The corresponding N1QL query that's generated is this:

SELECT META(`travel-sample`).id AS _ID, META(`travel-sample`).cas AS _CAS, `travel-sample`.* FROM `travel-sample` WHERE `icao` = \"LFBF\" AND `_class` = \"name.abhijitsarkar.javaee.travel.domain.Airport\"

However, as shown below, the sample bucket documents don't have a _class field. Thus the query doesn't find anything.

{
  "id": 1266,
  "type": "airport",
  "airportname": "Francazal",
  "city": "Toulouse",
  "country": "France",
  "faa": null,
  "icao": "LFBF",
  "tz": "Europe/Paris",
  "geo": {
    "lat": 43.545555,
    "lon": 1.3675,
    "alt": 535
  }
}

Thus, I'm forced to put a @Query("$SELECT_ENTITY$ WHERE icao = $1") on the method that produces the following N1QL, and expected result:

SELECT META(`travel-sample`).id AS _ID, META(`travel-sample`).cas AS _CAS, `travel-sample`.* FROM `travel-sample` WHERE icao = $1

There should be an option, probably repository level, and perhaps also query level, to not add the class name to generated queries


Affects: 2.0 M1, 2.0 RC1

Referenced from: commits f453441, 61731ae

2 votes, 7 watchers

@spring-projects-issues
Copy link
Author

Simon Baslé commented

At first, it doesn't seem that hard a feature to request... But ultimately repositories have to be opinionated on the way they interpret data in store in order to provide out of the box marshalling and unmarshalling!

Basically repositories are supposed to be a self-contained tool to deal with entities they persisted themselves. The _class field is instrumental to that (although it can be renamed in current snapshot). It is also necessary because the (un)marshalling internals are dealing with additional things like CAS, the document ID to choose where to store the data, etc...

Here your second query by chance only select airport documents, because they are the only ones to have an icao... Travel-sample contains no less that 4 different types of data. If you change the query to use a more generic field, then you're going to have a problem.

If you're not prepared to let the application using the Repository in charge of generating and persisting the data (with a possible delta of a data migration), then Repository is probably not the best abstraction for you.

What you could use is the CouchbaseTemplate's findByN1qlProjection to attempt a simpler unmarshalling, possibly even just a projection of a subset of the document (eg. in your example you want to unmarshall to a SimpleAirport that only contains the icao and airportname. You can there craft the query using the Couchbase SDK semantics.

@spring-projects-issues
Copy link
Author

Abhijit Sarkar commented

I think there's an easier way to get what I want and still fit with the existing framework. The classname is nothing but type. Acc. to this blog, there's already a way to rename the field. But the code, I forgot the class name, picks the value as the Java class. Instead, if you extend the @ Document annotation with a type field, the code could use that field value instead of the Java class. It makes sense to have the document type with the @ Document annotation

@spring-projects-issues
Copy link
Author

Mike Shindle commented

Instead of extending the @Document annotation, I solved the same problem by applying the currently existing @TypeAlias annotation. Applications can define their type key through configuration, and entities can define their alias using the existing spring data annotations. To make it work, I had to extend a bunch of SpringData Couchbase classes to incorporate the annotation scanning and remove the assumption that typeValue is the classname. I'll work on creating a pull request to incorporate the changes into the current stack instead of just extending classes unless extending @Document should be the design since we do not want to incorporate @TypeAlias in this manner

@spring-projects-issues
Copy link
Author

Abhijit Sarkar commented

IMHO, the whole point of Spring Data is to avoid writing boilerplate code. That's why faced with this issue, I just resorted to using Couchbase native Java client. They're reactive compliant, something Spring Data isn't and I can do a lot more using Observable than I can do using Stream

@spring-projects-issues
Copy link
Author

Sev Burmaka commented

Do you happen to have a link to the PR and affected code? I'm interested in this approach

@spring-projects-issues
Copy link
Author

Abhijit Sarkar commented

@Sev Burmaka I didn't submit a PR and the affected code is in the original question

@spring-projects-issues
Copy link
Author

Sev Burmaka commented

@Abhijit Sarkar sorry I was referring to the solution that @Mike Shindle described. TypeAlias does not currently seem to be supported as a method for manually specifying the typeValue and I'm not sure how else to circumvent this without large code changes

@spring-projects-issues
Copy link
Author

Eduard Dudar commented

Agree that there should be an option for exact type value to be provided. Current approach with full class name is not convenient or flexible. It may work fine for hobby-size projects but as soon as you have XX GBs of data in production and you need to move classes around (refactoring... that thing that happens very often in real-world projects) - you're totally screwed. Even if I override getTypeKey() with specific field, secondary indices are still created with a full class name as a value

@spring-projects-issues
Copy link
Author

Simon Baslé commented

@Eduard Dudar: note spring-data-couchbase isn't supposed to create indexes. there's a feature for that but it is opt-in and only meant for dev/test... If you don't want secondary indices, then maybe spring-data-couchbase isn't the best fit, as it is built around those.

The type information itself isn't going away, as it is there for a reason. The fact that Couchbase can store heterogeneous data in the same bucket is an additional challenge: how should methods like findAll only select the documents that are relevant to their associated repository? That was the initial idea behind using _class in query derivation.

But it could make sense for the query derivation not to select relevant couchbase documents based on the _class criteria (as it generates it from the repository's generic type, which could be irrelevant if there is a lot of polymorphism or refactoring).

Instead query derivation and methods like findAll should probably rely on a separate criteria, like the usual couchbase pattern of using a type field... (so something user-configurable, probably with an annotation on the document level).

Ultimately, support for a configurable string-to-class mapping could also be added (there's one in spring-data-mongo IIRC)

@spring-projects-issues
Copy link
Author

Eduard Dudar commented

Simon, I don't advocate to remove this info completely but just to make it a little more flexible. Document level annotation or even that solution you've mentioned from data-mongo seems to be a reasonable trade-off. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core support in: repository Repositories abstraction status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants