jakartaee / persistence

https://jakartaee.github.io/persistence/
Other
187 stars 55 forks source link

a clarification to field-level @Access #552

Closed gavinking closed 6 months ago

gavinking commented 6 months ago

and move footnotes into main text

sebersole commented 6 months ago

What do you think about clarifying "It is an error if a default access type cannot be determined"? I think the general approach for all providers is to look at @Id / @EmbeddedId, so maybe codify that?

Although, the question could then become why is this:

@Entity
class Book {
    @Id
    Integer id;
}

not functionally the same as

@Entity
@Access(FIELD)
class Book {
    @Id
    Integer id;
}

for the purpose of something like

@Entity
class Book {
    @Id
    Integer id;
    ...

    @Access(PROPERTY)
    String getIsbn() ...
}

The only distinction being implicit v. explicit in that case.

gavinking commented 6 months ago

It is an error if a default access type cannot be determined

For my part, I'm completely happy to add that. @lukasj WDYT?

Although, the question could then become...

So there's two things you could say, I suppose. Either:

Either option is self-consistent, and either option is fine by me.

sebersole commented 6 months ago

It is an error if a default access type cannot be determined

For my part, I'm completely happy to add that. @lukasj WDYT?

That is already in the original and in your PR. I was just saying we might consider adding the bit about @Id to explicitly note that that does in fact constitute a determination.

sebersole commented 6 months ago

So there's two things you could say, I suppose. Either:

I also don't much care which way we interpret that. But having it explicit and consistent is a win imo

gavinking commented 6 months ago

So there's two things you could say, I suppose. Either:

I also don't much care which way we interpret that. But having it explicit and consistent is a win imo

Let's migrate this discussion to issue #558.